Make WordPress Core

Opened 7 years ago

Closed 3 months ago

#44628 closed defect (bug) (fixed)

Repair DB rehashes password to md5

Reported by: yaniiliev's profile yani.iliev Owned by: johnbillion's profile johnbillion
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch needs-testing
Focuses: Cc:

Description

How to replicate:

Corrupt the database:

truncate $wpdb->options;
insert into $wpdb->options (option_name, option_value) values('siteurl', 'http://localhost');

Navigate to http://localhost/ and repair the database.

Observe user_pass for all users in $wpdb->users it is now md5 hash.

Attachments (1)

44628.diff (1.0 KB) - added by jbcomte35 6 years ago.
Diff file

Download all attachments as: .zip

Change History (10)

#1 @yani.iliev
7 years ago

The code that rehashes the password is in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/upgrade.php?annotate=blame#L889

upgrade_110 is used to convert plaintext passwords to md5, however, the hash has changed in the most recent versions of WordPress and the regex needs updating.
wp_check_password uses

<?php
if ( strlen($hash) <= 32 ) {

to check if the hash is md5 but ideally it should be === because md5 is always 32 characters.
https://core.trac.wordpress.org/browser/tags/4.9.7/src/wp-includes/pluggable.php#L2237

if password length < 32 hash password with md5

if password length === 32 check if the string has all valid md5 characters

if the password has valid md5 characters, assume it is md5 checksum

else assume it is not hashed and md5 hash it

if the password is > 32 characters do not hash it.

#2 @pento
6 years ago

  • Component changed from Database to Upgrade/Install
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version trunk deleted

#3 @jbcomte35
6 years ago

I'm on it, i'll push a .diff file within few days

Last edited 6 years ago by jbcomte35 (previous) (diff)

@jbcomte35
6 years ago

Diff file

#4 @jbcomte35
6 years ago

  • Keywords has-patch added; needs-patch removed

#5 @jbcomte35
6 years ago

  • Keywords needs-testing added

#6 @bookdude13
5 years ago

Hi everyone! This is my first post, so let me know if I'm doing anything wrong.

I tried to reproduce this using the steps above, except I did the commands directly in phpMyAdmin. This left me with a blank site, no db corruption. I then put

 python
  define( 'WP_DEBUG', true );

in my wp-config.php file and tried repairing the database at http://localhost/wordpress-svn/src/wp-admin/maint/repair.php. The repairs didn't change the wp_users hashes. I also tried corrupting the wp_options data file directly with a few values, and this either caused the whole db server to crash or didn't make noticeable impacts. Repairing afterwards didn't change the hashes.

It looks like the function that was causing the issue and that this patch addresses is only called with db version < 2541 (WP 1.5?). See wp-admin\includes\upgrade.php, lines 725-728. The current db version is around 47018.

Tl;dr I couldn't reproduce on trunk, the repro steps should be updated, and future testing should be done w/ an older version of wordpress.

#7 @johnbillion
7 months ago

  • Milestone changed from Future Release to 6.8
  • Owner set to johnbillion
  • Status changed from new to accepted

#8 @johnbillion
6 months ago

The fix for this is included in the PR on #21022.

#9 @johnbillion
3 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 59828:

Security: Switch to using bcrypt for hashing user passwords and BLAKE2b for hashing application passwords and security keys.

Passwords and security keys that were saved in prior versions of WordPress will continue to work. Each user's password will be opportunistically rehashed and resaved when they next subsequently log in using a valid password.

The following new functions have been introduced:

  • wp_password_needs_rehash()
  • wp_fast_hash()
  • wp_verify_fast_hash()

The following new filters have been introduced:

  • password_needs_rehash
  • wp_hash_password_algorithm
  • wp_hash_password_options

Props ayeshrajans, bgermann, dd32, deadduck169, desrosj, haozi, harrym, iandunn, jammycakes, joehoyle, johnbillion, mbijon, mojorob, mslavco, my1xt, nacin, otto42, paragoninitiativeenterprises, paulkevan, rmccue, ryanhellyer, scribu, swalkinshaw, synchro, th23, timothyblynjacobs, tomdxw, westi, xknown.

Additional thanks go to the Roots team, Soatok, Calvin Alkan, and Raphael Ahrens.

Fixes #21022, #44628

Note: See TracTickets for help on using tickets.