Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-12983

UCP preferences, Display posts ordering by: input is not properly validated

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.0-RC3
    • Fix Version/s: 3.1.0-RC4
    • Labels:
      None

      Description

      Hi,

      Register user is able to trigger a general error by supplying unvalidated data resulting in an invalid SQL query.

      First of all, I do understand the end result ( getting the admin email address ) is part of system, but I would like to advise against it.

      How to reproduce:

      UCP->Board preferences->Edit display options->Display posts ordering by
      Input is not properly validated, user can have the system to insert any single characters into the database. Example: 'x'

      After the user visit any topics a general error will be seen including admin password:

      General Error
      SQL ERROR [ mysqli ]

      You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC LIMIT 10' at line 7 [1064]

      An SQL error occurred while fetching this page. Please contact the Board Administrator if this problem persists.

      In viewtopics.php line 38 the previous unvalidated input used as $default_sort_key -> $sort_key and later on at line 971 :
      $sql_sort_order = $sort_by_sql[$sort_key] . ' ' . $direction

      As the 'x' array entry is nonexistent, the final sql query will be invalid:
      SELECT p.post_id FROM phpbb_posts p WHERE p.topic_id = 2 AND p.post_approved = 1 ORDER BY ASC.

      The issue might be seen as a simple functional bug, but I strongly believe getting the admin email gives a huge advantage for the attacker.

      Regards,
      Marcell

        Activity

        Hide
        EXreaction EXreaction [X] (Inactive) added a comment -

        The validation issue should be resolved, but the admin email address is a necessity to allow users to contact owners in case of an unknown error.

        None of this is a security issue, can we move this to our normal tracker?

        Show
        EXreaction EXreaction [X] (Inactive) added a comment - The validation issue should be resolved, but the admin email address is a necessity to allow users to contact owners in case of an unknown error. None of this is a security issue, can we move this to our normal tracker?
        Hide
        nickvergessen Joas Schilling added a comment -

        Also note that we check the length of the input, so only 1 character is allowed and the value is not used directly in the query, so no SQL injection is possible.
        We should still fix the issue. Do we copy this to a normal ticket now? Nils Adermann

        Show
        nickvergessen Joas Schilling added a comment - Also note that we check the length of the input, so only 1 character is allowed and the value is not used directly in the query, so no SQL injection is possible. We should still fix the issue. Do we copy this to a normal ticket now? Nils Adermann

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            fodor fodor [X] (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development