Bad usage of the PHP hash_equals function

Created on 3 December 2024, 18 days ago

After discussing this with the security team, I'm publicly reposting the following issue :

Hi,

The core of drupal is using the PHP function hash_equals in multiple places.
The way this function is used does not always respect the api contract of the function
We can take Drupal\user\Controller\UserController::validatePathParameters() as an example :

protected function validatePathParameters(UserInterface $user, int $timestamp, string $hash, int $timeout = 0): bool {
    $current = \Drupal::time()->getRequestTime();
    $timeout_valid = ((!empty($timeout) && $current - $timestamp < $timeout) || empty($timeout));
    return ($timestamp >= $user->getLastLoginTime()) && $timestamp <= $current && $timeout_valid && hash_equals($hash, user_pass_rehash($user, $timestamp));
  }
  1. The length of the 2 parameters must be the same, otherwise the function return immediatly. In my example, the lengths are not check to be the same which can allow an attacker to determine the lenght of the known string by checking the timing of the comparaison => this is not really an issue since Drupal is open source and the length are known
  2. I am less sure about this one after checking the C implementation of PHP (https://github.com/php/php-src/blob/PHP-8.4.0/ext/hash/hash.c#L1114 and https://github.com/php/php-src/blob/PHP-8.4.0/main/safe_bcmp.c) : if we read the doc (https://www.php.net/manual/en/function.hash-equals.php), it is important to supply the known string first. In my example (and in multiple other places), the known string is supplied as the second argument

I known this is a very remote security issue. But since the goal of using hash_equals() is to mitigate timing attacks, I think we can pursue every little details.

Thanks.

📌 Task
Status

Active

Version

10.3

Component

other

Created by

🇨🇭Switzerland epieddy

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024