From 7d37b650a4008e80135ac8898fe973baad8f4e92 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 4 Feb 2021 21:20:50 +0100 Subject: [PATCH] [ticket/16697] Remove $CP$ prefix when updating hashes and support phpBB2 check The $CP$ prefix is not part of the actual password hash. phpBB2 passwords converted do currently include a phpass hash of the md5 of the password. Make sure these are correctly checked. PHPBB3-16697 --- .../console/command/fixup/update_hashes.php | 3 ++- phpBB/phpbb/cron/task/core/update_hashes.php | 3 ++- phpBB/phpbb/passwords/manager.php | 16 ++++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/console/command/fixup/update_hashes.php b/phpBB/phpbb/console/command/fixup/update_hashes.php index 9a0e9bc798..8a2ef60771 100644 --- a/phpBB/phpbb/console/command/fixup/update_hashes.php +++ b/phpBB/phpbb/console/command/fixup/update_hashes.php @@ -99,7 +99,8 @@ class update_hashes extends \phpbb\console\command\command while ($row = $this->db->sql_fetchrow($result)) { - $new_hash = $this->passwords_manager->hash($row['user_password'], array($this->default_type)); + $old_hash = preg_replace('/^\$CP\$/', '', $row['user_password']); + $new_hash = $this->passwords_manager->hash($old_hash, array($this->default_type)); $sql = 'UPDATE ' . USERS_TABLE . " SET user_password = '" . $this->db->sql_escape($new_hash) . "' diff --git a/phpBB/phpbb/cron/task/core/update_hashes.php b/phpBB/phpbb/cron/task/core/update_hashes.php index 9e938f74dd..3348a4576e 100644 --- a/phpBB/phpbb/cron/task/core/update_hashes.php +++ b/phpBB/phpbb/cron/task/core/update_hashes.php @@ -106,7 +106,8 @@ class update_hashes extends \phpbb\cron\task\base while ($row = $this->db->sql_fetchrow($result)) { - $new_hash = $this->passwords_manager->hash($row['user_password'], array($this->default_type)); + $old_hash = preg_replace('/^\$CP\$/', '', $row['user_password']); + $new_hash = $this->passwords_manager->hash($old_hash, array($this->default_type)); // Increase number so we know that users were selected from the database $affected_rows++; diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index 54e6dce4be..616704304d 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -382,11 +382,22 @@ class manager * @param array $stored_hash_type An array containing the hash types * as described by stored password hash * @param string $hash Stored password hash + * @param bool $skip_phpbb2_check True if phpBB2 password check should be skipped * * @return bool True if password is correct, false if not */ - public function check_combined_hash($password, $stored_hash_type, $hash) + public function check_combined_hash($password, $stored_hash_type, $hash, bool $skip_phpbb2_check = false) { + // Special case for passwords converted from phpBB2: + // These could be phpass(md5(password)) and hence already be double + // hashed. For these, try to also check combined hash output of + // md5 version of supplied password. + $is_valid_phpbb2_pass = false; + if (!$skip_phpbb2_check) + { + $is_valid_phpbb2_pass = $this->check_combined_hash(md5($password), $stored_hash_type, $hash, true); + } + $i = 0; $data = array( 'prefix' => '$', @@ -402,6 +413,7 @@ class manager $password = str_replace($rebuilt_hash, '', $cur_hash); $i++; } - return ($hash === $this->helper->combine_hash_output($data, 'hash', $password)); + + return hash_equals($hash, $this->helper->combine_hash_output($data, 'hash', $password)) || $is_valid_phpbb2_pass; } }