Password hash update implementation causes user password to get incorrectly overwritten

[johnbillion]

Hi Carl,

I'm working on implementing the switch to using bcrypt for password hashing in WordPress core. As part of this work I'm making sure that existing plugins that implement bcrypt (and other hashing algorithms) are compatible with the change.

While looking at Passwords Evolved I've identified an issue that will cause a user's password to be overwritten with an unexpected value whenever wp_check_password() is used to check something other than the user's password. This includes:

• When a valid Application Password is used that was hashed with phpass (ie. prior to activating this plugin).
• When wp_check_password() is used to check something other than the user's password, for example an authentication code. There are existing popular plugins that do this, including Two Factor, Solid Security, and others.

In these cases Passwords Evolved will rehash the password for the user each time a successful check is made. The user will need to reset their actual password each time after this happens, unless the password coincidentally matches the one being checked.

The crux of the problem is that the wp_check_password() implementation in Passwords Evolved assumes the function is only used to check the user password of the given user, which is incorrect. The password can be for anything, and it isn't always associated with a user account. Passwords Evolved calls wp_set_password() which sets the password for the user, however the password being checked isn't necessarily the user's password. Here is the problematic code.

In my implementation for WordPress core, the logic for rehashing a password lives outside of wp_check_password() for this reason. I'm hoping to get this into 6.8, but we'll see. Correcting the rehashing in Passwords Evolved might not be possible, so I wonder if it would be best to wait until the change is merged in a new release of core. That will allow the rehashing to be handled, and then Password Evolved can concentrate on optionally using argon2i and argon2id, which isn't going to be supported natively in core.

[carlalexander]

Hi @johnbillion! 👋

I'll try to check this out soon. Thank you so much for the detailed report 💖

[carlalexander]

Yeah, I think it's better to wait until your change is merged into core. Even the current wp_check_password function in core rehashes the password if it's not using the right encryption.

[johnbillion]

I agree 👍