Details

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

      Description

      This is very theoretical but I think you'll agree with the conclusion I make.

      The "Manage “Remember Me” login keys" section in the UCP lists the current users login keys, currently this displays the ID (the login key), last IP and last use time.

      The login key is an MD5 hash of the cookie stored in the users browser, a theoretical XSS attack against phpBB could allow an attacker to steal the login keys for a user. Alternatively a user could inadvertently publish the login keys without realising the consequences.

      An attacker after stealing the login keys can brute force the MD5 hash, given a single GPU this is possible in < 30 years. Applying parallel processing this time could be reduced to under a month (less than the default validity period.)

      Given the login key is useless to the user, I don't see a valid use case to reveal it and possibly allowing it to be collected by an attacker. A savvy user could compare the cookies and login keys to work out which browser is which in their list. To aid usability the current login key should be highlighted somehow within the list.

        Activity

        Hide
        nickvergessen Joas Schilling added a comment -

        The potential problem I see here is, that we need a unique key for the check box.
        But apart from the key_id the table does not contain anything that is unique.
        We could try to abuse the last_login + ip, which should be unique in a normal world.
        Just wanted to share my concerns before preparing a patch.

        Show
        nickvergessen Joas Schilling added a comment - The potential problem I see here is, that we need a unique key for the check box. But apart from the key_id the table does not contain anything that is unique. We could try to abuse the last_login + ip, which should be unique in a normal world. Just wanted to share my concerns before preparing a patch.
        Hide
        ToonArmy Chris Smith added a comment -

        That's a very good point. However you cannot use the other table data as that's subject to change.

        I'd probably add a traditional PK column to that table and using the key only to verify remember me logins, using this method you could even change the remember me login process so that it's not open to timing attacks.

        I'm quite surprised 3.1 doesn't use a constant time comparison for passwords either: https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/phpbb/passwords/driver/bcrypt.php#L71

        Show
        ToonArmy Chris Smith added a comment - That's a very good point. However you cannot use the other table data as that's subject to change. I'd probably add a traditional PK column to that table and using the key only to verify remember me logins, using this method you could even change the remember me login process so that it's not open to timing attacks. I'm quite surprised 3.1 doesn't use a constant time comparison for passwords either: https://github.com/phpbb/phpbb/blob/develop-ascraeus/phpBB/phpbb/passwords/driver/bcrypt.php#L71
        Hide
        Marc Marc added a comment -

        What about just showing part of the login key md5 hash? 8 or maybe 12 digits from the beginning or end should be enough to prevent any collisions for a user while also preventing someone from being able to retrieve a full hash. Deleting the login keys would then be done using a like expression in the database query.

        Show
        Marc Marc added a comment - What about just showing part of the login key md5 hash? 8 or maybe 12 digits from the beginning or end should be enough to prevent any collisions for a user while also preventing someone from being able to retrieve a full hash. Deleting the login keys would then be done using a like expression in the database query.
        Hide
        naderman Nils Adermann added a comment -

        Chris: http://security.stackexchange.com/questions/9192/timing-attacks-on-password-hashes - it's not really necessary to compare hashes in constant time, but we changed that now anyway: https://github.com/phpbb/phpbb/pull/3056

        Show
        naderman Nils Adermann added a comment - Chris: http://security.stackexchange.com/questions/9192/timing-attacks-on-password-hashes - it's not really necessary to compare hashes in constant time, but we changed that now anyway: https://github.com/phpbb/phpbb/pull/3056

          People

          • Assignee:
            Marc Marc
            Reporter:
            ToonArmy Chris Smith
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development