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

Loose string comparison during new password activation

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.11
    • Fix Version/s: 3.0.12-RC1
    • Labels:
      None

      Description

      Although request_var() takes care of casting user input to the appropriate type, when comparing strings in a security context, it is required to use strict comparison (===). This is because e.g. "10" == "1e1" evaluates to true which might weaken security properties (e.g. when comparing to a random string).

        Issue Links

          Activity

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

          Was this changed for 3.1-dev then? In phpbb_check_password in 3.1-dev I have:

          return (_hash_crypt_private($password, $hash, $itoa64) === $hash) ? true : false;

          Show
          EXreaction EXreaction [X] (Inactive) added a comment - Was this changed for 3.1-dev then? In phpbb_check_password in 3.1-dev I have: return (_hash_crypt_private($password, $hash, $itoa64) === $hash) ? true : false;
          Hide
          bantu Andreas Fischer added a comment -

          I don't understand your question.

          Show
          bantu Andreas Fischer added a comment - I don't understand your question.
          Hide
          bantu Andreas Fischer added a comment -

          To clearify: This is about uses such as

          $user_row['user_actkey'] != $key

          Show
          bantu Andreas Fischer added a comment - To clearify: This is about uses such as $user_row['user_actkey'] != $key
          Hide
          imkingdavid David King added a comment - - edited

          Nathan, this issue is specifically tied to this topic: https://www.phpbb.com/community/viewtopic.php?f=44&t=2169160 and is present in both olympus and develop.

          EDIT: To clarify, the "vulnerability" is very weak. Basically, it allows an attacker to, with a very low likelihood, use 1 or 0 as the activation key for resetting a user's password because some strings can be evaluated as numbers using loose comparison (==). This ultimately can lock a user out of their account by changing their password to a temporary one, but in the end, the user will still have the temporary password in their email, which the attacker cannot access. As such, this does not result in the attacker being able to gain the user's account, unless the attacker is able to access the user's email, in which case he would not need to use this "vulnerability" anyway, as both the activation key and the temporary password are sent in the email. It is more of an annoyance than a vulnerability. However, it is something we should fix.

          Show
          imkingdavid David King added a comment - - edited Nathan, this issue is specifically tied to this topic: https://www.phpbb.com/community/viewtopic.php?f=44&t=2169160 and is present in both olympus and develop. EDIT: To clarify, the "vulnerability" is very weak. Basically, it allows an attacker to, with a very low likelihood, use 1 or 0 as the activation key for resetting a user's password because some strings can be evaluated as numbers using loose comparison (==). This ultimately can lock a user out of their account by changing their password to a temporary one, but in the end, the user will still have the temporary password in their email, which the attacker cannot access. As such, this does not result in the attacker being able to gain the user's account, unless the attacker is able to access the user's email, in which case he would not need to use this "vulnerability" anyway, as both the activation key and the temporary password are sent in the email. It is more of an annoyance than a vulnerability. However, it is something we should fix.
          Hide
          bantu Andreas Fischer added a comment -

          This ticket should obviously be about all "loose string comparisons in a security context", not just the new password activation one. $user_row['user_actkey'] != $key however seems to be the only occurance of this issue so far.

          Also, FYI: This ticket will be made public sooner or later.

          Show
          bantu Andreas Fischer added a comment - This ticket should obviously be about all "loose string comparisons in a security context", not just the new password activation one. $user_row ['user_actkey'] != $key however seems to be the only occurance of this issue so far. Also, FYI: This ticket will be made public sooner or later.
          Hide
          EXreaction EXreaction [X] (Inactive) added a comment -

          Oops, I must have read this too fast on my phone and thought it was about password comparison, not during password activation.

          Show
          EXreaction EXreaction [X] (Inactive) added a comment - Oops, I must have read this too fast on my phone and thought it was about password comparison, not during password activation.

            People

            • Assignee:
              bantu Andreas Fischer
              Reporter:
              imkingdavid David King
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development