Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#63412 new defect (bug)

Bcrypt - Cannot Verify Password Hashes

Reported by: aaron13223's profile aaron13223 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.8
Component: Login and Registration Keywords: 2nd-opinion
Focuses: Cc:

Description

Hi There,

Some bcrypt hashes cannot be verified likely due to an escape character issue.

One example of hash is:

$2b$10$AmZawb7AujOXxowQuhUk2.sg69pZge8uG.zdbIc.FMRmz54Op8x9u

The password for this hash is: A3oc5tq9'U&d

If you try to verify this anywhere, it will verify like it should but it doesn't work in WP. We also noticed this could be an escape issue since if we try to verify this with a simple php script:

<?php
// The password to check
$password = "A3oc5tq9'U&d";

// The bcrypt hash to check against
$hash = "$2b$10$AmZawb7AujOXxowQuhUk2.sg69pZge8uG.zdbIc.FMRmz54Op8x9u";

// Verify the password against the hash
if (password_verify($password, $hash)) {
    echo "correct";
} else {
    echo "incorrect";
}

This will throw out an error for undefined variable, we need to either wrap the hash in single quotes or escape it like so:

$hash = "$2b\$10\$AmZawb7AujOXxowQuhUk2.sg69pZge8uG.zdbIc.FMRmz54Op8x9u";

Storing this hash directly in the database with escaped character does not help either.

We found this since we are running an import of users from another platform which also uses bcrypt for hashing passwords. (JS bcrypt library 5.1.1 for reference)

Please let me know what you think and if this is even a WordPress Specific issue.

Change History (3)

#1 @dd32
6 weeks ago

  • Keywords 2nd-opinion added

Hi @aaron13223 and welcome to Trac,

Firstly, as the hash string contains $ you do need to escape it within PHP string literals, either as '$2b...' or "\$2b..." as noted, this is not a bug as such, just a language construct. PHP Strings can contain $ without issue, it's just the PHP Parser that interpolates the variables.

Next, WordPress unfortunately requires some strings be escaped when used for certain operations, for example: wp_set_password( wp_slash( "A3oc5tq9'U&d" ), $user_id );.

wp_slash() is the same as addslashes() and would escape A3oc5tq9'U&d to A3oc5tq9\'U&d.

This code would return truthful (Your strings from the ticket)

password_verify( "A3oc5tq9'U&d", '$2b$10$AmZawb7AujOXxowQuhUk2.sg69pZge8uG.zdbIc.FMRmz54Op8x9u' );

However.. WordPress is effectively running:

$hash = '$2b$10$AmZawb7AujOXxowQuhUk2.sg69pZge8uG.zdbIc.FMRmz54Op8x9u';

password_verify( wp_slash( "A3oc5tq9'U&d" ), $hash );
which is:
password_verify( "A3oc5tq9\'U&d", $hash );

We found this since we are running an import of users from another platform which also uses bcrypt for hashing passwords. (JS bcrypt library 5.1.1 for reference)

So ultimately the issue at hand here, is that while WordPress uses bcrypt hashes, the input password is being escaped differently between your application and WordPress.

The Password hashes WordPress generates using bcrypt are either $wp... or $P$.... bcrypt hashes, which both contain some extra customizations, but WordPress does support fallback to "native" bcrypt.

Those customizations affect passwords over 72 char (Which are run through additional HMAC hashing) and no support for >4096 char passwords. See wp_hash_password() and wp_check_password() for those: https://github.com/WordPress/wordpress-develop/blob/20e3fdd09fd1bbcea5e712117474d0945af528ea/src/wp-includes/pluggable.php#L2648-L2781

I suspect you'll be able to use the check_password filter in a plugin to alter the behaviour, something like this is probably going to resolve it:

<?php

// Support passwords from JS bcrypt which did not slash the password.
add_filter( 'check_password', function( $check, $password, $hash ) {
   if ( ! $check && str_starts_with( $hash, '$2b' ) && str_contains( $hash, "'" ) ) {
      $check = password_verify( wp_unslash( $password ), $hash );
   }

   return $check;
}, 10, 3 );

I don't think this is a bug per-se, but it might be worth adding back-compat support to WordPress for this edge-case, where non-wordpress-generated-hashes are the password and it's the non-slashed password inside that hash.

Last edited 6 weeks ago by dd32 (previous) (diff)

#2 @aaron13223
6 weeks ago

Wow Dion, you are right. It was pure happen chance that I checked this password in particular haha.

No matter how many hashes are generated for that particular password, it does not work until the backslash is added to it while generating the hash.

The filter works as expected, just fixed a typo in str_contains():

<?php

// Support passwords from JS bcrypt which did not slash the password.
function backwards_compatible_bcrypt_passwords( $check, $password, $hash ){

    if ( ! $check && str_starts_with( $hash, '$2b' ) && str_contains( $password, "'" ) ) {
        $check = password_verify( wp_unslash( $password ), $hash );
    }
    
    return $check;
}
add_filter( 'check_password', 'backwards_compatible_bcrypt_passwords', 10, 3 );

Thanks so much for your help!

Additionally, should I mark this as "wontfix" or similar or leave it as is? Let me know. Thanks a lot again!

#3 @dd32
6 weeks ago

just fixed a typo in str_contains():

:facepalm: Teaches me right for coding in a Trac <textarea>.

I think this ticket should remain open until someone who has worked with passwords can chime in on:

it might be worth adding back-compat support to WordPress for this edge-case, where non-wordpress-generated-hashes are the password and it's the non-slashed password inside that hash.

I attempted to find a ticket that this is suggested in, but I wasn't able to (too many search results), so it might have been discussed already.

I did find #24367 which suggests

we need to go back to storing a hash of the slashed password. Yes, this is stupid, and we ought to fix it, but right now let's handle the bug.

and #34297 which mentions breakage of slashed passwords, but nothing for compatibility for passwords generated outside of WordPress with slashes (Although if another dev feels like closing this as a duplicate of that one, go for it)

Note: See TracTickets for help on using tickets.