Commit 6dd4b186 authored by alexpott's avatar alexpott

Issue #2400197 by pwolanin, fgm, znerol, neclimdul, mpdonadio, thekevinday,...

Issue #2400197 by pwolanin, fgm, znerol, neclimdul, mpdonadio, thekevinday, Fabianx: Harden the security where hash values are compared
parent 7e245c98
......@@ -41,6 +41,13 @@ public static function randomBytes($count) {
$bytes .= openssl_random_pseudo_bytes($missing_bytes);
}
// If OpenSSL is not available, we can use mcrypt. On Windows, this will
// transparently pull from CryptGenRandom. On Unix-based systems, it will
// read from /dev/urandom as expected.
elseif (function_exists(('mcrypt_create_iv')) && defined('MCRYPT_DEV_URANDOM')) {
$bytes .= mcrypt_create_iv($count, MCRYPT_DEV_URANDOM);
}
// Else, read directly from /dev/urandom, which is available on many *nix
// systems and is considered cryptographically secure.
elseif ($fh = @fopen('/dev/urandom', 'rb')) {
......@@ -125,6 +132,49 @@ public static function hashBase64($data) {
return str_replace(['+', '/', '='], ['-', '_', ''], $hash);
}
/**
* Compares strings in constant time.
*
* @param string $known_string
* The expected string.
* @param string $user_string
* The user supplied string to check.
*
* @return bool
* Returns TRUE when the two strings are equal, FALSE otherwise.
*/
public static function hashEquals($known_string, $user_string) {
if (function_exists('hash_equals')) {
return hash_equals($known_string, $user_string);
}
else {
// Backport of hash_equals() function from PHP 5.6
// @see https://github.com/php/php-src/blob/PHP-5.6/ext/hash/hash.c#L739
if (!is_string($known_string)) {
trigger_error(sprintf("Expected known_string to be a string, %s given", gettype($known_string)), E_USER_WARNING);
return FALSE;
}
if (!is_string($user_string)) {
trigger_error(sprintf("Expected user_string to be a string, %s given", gettype($user_string)), E_USER_WARNING);
return FALSE;
}
$known_len = strlen($known_string);
if ($known_len !== strlen($user_string)) {
return FALSE;
}
// This is security sensitive code. Do not optimize this for speed.
$result = 0;
for ($i = 0; $i < $known_len; $i++) {
$result |= (ord($known_string[$i]) ^ ord($user_string[$i]));
}
return $result === 0;
}
}
/**
* Returns a URL-safe, base64 encoded string of highly randomized bytes.
*
......
......@@ -251,7 +251,9 @@ public function check($password, $hash) {
default:
return FALSE;
}
return ($computed_hash && $stored_hash === $computed_hash);
// Compare using hashEquals() instead of === to mitigate timing attacks.
return $computed_hash && Crypt::hashEquals($stored_hash, $computed_hash);
}
/**
......
......@@ -7,6 +7,7 @@
namespace Drupal\toolbar\Controller;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Ajax\AjaxResponse;
use Drupal\Core\Controller\ControllerBase;
......@@ -52,7 +53,8 @@ public function subtreesAjax() {
* The access result.
*/
public function checkSubTreeAccess($hash) {
return AccessResult::allowedIf($this->currentUser()->hasPermission('access toolbar') && $hash == _toolbar_get_subtrees_hash()[0])->cachePerPermissions();
$expected_hash = _toolbar_get_subtrees_hash()[0];
return AccessResult::allowedIf($this->currentUser()->hasPermission('access toolbar') && Crypt::hashEquals($expected_hash, $hash))->cachePerPermissions();
}
}
......@@ -7,6 +7,7 @@
namespace Drupal\user;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Entity\ContentEntityForm;
use Drupal\Core\Entity\EntityConstraintViolationListInterface;
use Drupal\Core\Entity\EntityManagerInterface;
......@@ -127,7 +128,7 @@ public function form(array $form, FormStateInterface $form_state) {
// one-time link and have the token in the URL. Store this in $form_state
// so it persists even on subsequent Ajax requests.
if (!$form_state->get('user_pass_reset')) {
$user_pass_reset = $pass_reset = isset($_SESSION['pass_reset_' . $account->id()]) && (\Drupal::request()->query->get('pass-reset-token') == $_SESSION['pass_reset_' . $account->id()]);
$user_pass_reset = isset($_SESSION['pass_reset_' . $account->id()]) && Crypt::hashEquals($_SESSION['pass_reset_' . $account->id()], \Drupal::request()->query->get('pass-reset-token'));
$form_state->set('user_pass_reset', $user_pass_reset);
}
......
......@@ -7,6 +7,7 @@
namespace Drupal\user\Controller;
use Drupal\Component\Utility\Crypt;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Datetime\DateFormatterInterface;
......@@ -123,7 +124,7 @@ public function resetPass($uid, $timestamp, $hash) {
drupal_set_message($this->t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'), 'error');
return $this->redirect('user.pass');
}
elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && ($hash === user_pass_rehash($user, $timestamp))) {
elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && Crypt::hashEquals($hash, user_pass_rehash($user, $timestamp))) {
$expiration_date = $user->getLastLoginTime() ? $this->dateFormatter->format($timestamp + $timeout) : NULL;
return $this->formBuilder()->getForm('Drupal\user\Form\UserPasswordResetForm', $user, $expiration_date, $timestamp, $hash);
}
......@@ -198,7 +199,7 @@ public function confirmCancel(UserInterface $user, $timestamp = 0, $hashed_pass
$account_data = $this->userData->get('user', $user->id());
if (isset($account_data['cancel_method']) && !empty($timestamp) && !empty($hashed_pass)) {
// Validate expiration and hashed password/login.
if ($timestamp <= $current && $current - $timestamp < $timeout && $user->id() && $timestamp >= $user->getLastLoginTime() && $hashed_pass == user_pass_rehash($user, $timestamp)) {
if ($timestamp <= $current && $current - $timestamp < $timeout && $user->id() && $timestamp >= $user->getLastLoginTime() && Crypt::hashEquals($hashed_pass, user_pass_rehash($user, $timestamp))) {
$edit = array(
'user_cancel_notify' => isset($account_data['cancel_notify']) ? $account_data['cancel_notify'] : $this->config('user.settings')->get('notify.status_canceled'),
);
......
......@@ -40,7 +40,7 @@
if (Settings::get('rebuild_access', FALSE) ||
($request->get('token') && $request->get('timestamp') &&
((REQUEST_TIME - $request->get('timestamp')) < 300) &&
($request->get('token') === Crypt::hmacBase64($request->get('timestamp'), Settings::get('hash_salt')))
Crypt::hashEquals(Crypt::hmacBase64($request->get('timestamp'), Settings::get('hash_salt')), $request->get('token'))
)) {
// Clear the APC cache to ensure APC class loader is reset.
if (function_exists('apc_clear_cache')) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment