Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #2455079 by klausi, David_Rothstein, pwolanin, benjy, Berdir: Password reset URL access bypass fixes from SA-CORE-2015-001 need to be ported to Drupal 8

8.0.x
Alex Pott 2015-03-19 12:40:07 +00:00
parent 531f95eb45
commit 8e54eca05a
4 changed files with 59 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/modules/user/src/Controller/UserController.php
View File

@ -122,7 +122,7 @@ class UserController extends ControllerBase {
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.'));
return $this->redirect('user.pass');
}
elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && ($hash === user_pass_rehash($user->getPassword(), $timestamp, $user->getLastLoginTime()))) {
elseif ($user->isAuthenticated() && ($timestamp >= $user->getLastLoginTime()) && ($timestamp <= $current) && ($hash === user_pass_rehash($user->getPassword(), $timestamp, $user->getLastLoginTime(), $user->id()))) {
$expiration_date = $user->getLastLoginTime() ? $this->dateFormatter->format($timestamp + $timeout) : NULL;
return $this->formBuilder()->getForm('Drupal\user\Form\UserPasswordResetForm', $user, $expiration_date, $timestamp, $hash);
}
@ -197,7 +197,7 @@ class UserController extends ControllerBase {
$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->getPassword(), $timestamp, $user->getLastLoginTime())) {
if ($timestamp <= $current && $current - $timestamp < $timeout && $user->id() && $timestamp >= $user->getLastLoginTime() && $hashed_pass == user_pass_rehash($user->getPassword(), $timestamp, $user->getLastLoginTime(), $user->id())) {
$edit = array(
'user_cancel_notify' => isset($account_data['cancel_notify']) ? $account_data['cancel_notify'] : $this->config('user.settings')->get('notify.status_canceled'),
);

14
core/modules/user/src/Tests/UserCancelTest.php
View File

@ -56,7 +56,7 @@ class UserCancelTest extends WebTestBase {
// Attempt bogus account cancellation request confirmation.
$timestamp = $account->getLastLoginTime();
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()));
$this->assertResponse(403, 'Bogus cancelling request rejected.');
$account = user_load($account->id());
$this->assertTrue($account->isActive(), 'User account was not canceled.');
@ -153,14 +153,14 @@ class UserCancelTest extends WebTestBase {
// Attempt bogus account cancellation request confirmation.
$bogus_timestamp = $timestamp + 60;
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime(), $account->id()));
$this->assertText(t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'Bogus cancelling request rejected.');
$account = user_load($account->id());
$this->assertTrue($account->isActive(), 'User account was not canceled.');
// Attempt expired account cancellation request confirmation.
$bogus_timestamp = $timestamp - 86400 - 60;
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$bogus_timestamp/" . user_pass_rehash($account->getPassword(), $bogus_timestamp, $account->getLastLoginTime(), $account->id()));
$this->assertText(t('You have tried to use an account cancellation link that has expired. Please request a new one using the form below.'), 'Expired cancel account request rejected.');
$account = user_load($account->id(), TRUE);
$this->assertTrue($account->isActive(), 'User account was not canceled.');
@ -198,7 +198,7 @@ class UserCancelTest extends WebTestBase {
$this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.');
// Confirm account cancellation request.
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()));
$account = user_load($account->id(), TRUE);
$this->assertTrue($account->isBlocked(), 'User has been blocked.');
@ -253,7 +253,7 @@ class UserCancelTest extends WebTestBase {
$this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.');
// Confirm account cancellation request.
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()));
$account = user_load($account->id(), TRUE);
$this->assertTrue($account->isBlocked(), 'User has been blocked.');
@ -310,7 +310,7 @@ class UserCancelTest extends WebTestBase {
$this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.');
// Confirm account cancellation request.
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()));
$this->assertFalse(user_load($account->id(), TRUE), 'User is not found in the database.');
// Confirm that user's content has been attributed to anonymous user.
@ -379,7 +379,7 @@ class UserCancelTest extends WebTestBase {
$this->assertText(t('A confirmation request to cancel your account has been sent to your email address.'), 'Account cancellation request mailed message displayed.');
// Confirm account cancellation request.
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()));
$this->drupalGet("user/" . $account->id() . "/cancel/confirm/$timestamp/" . user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()));
$this->assertFalse(user_load($account->id(), TRUE), 'User is not found in the database.');
// Confirm that user's content has been deleted.

45
core/modules/user/src/Tests/UserPasswordResetTest.php
View File

@ -8,6 +8,7 @@
namespace Drupal\user\Tests;
use Drupal\simpletest\WebTestBase;
use Drupal\user\Entity\User;
/**
* Ensure that password reset methods work as expected.
@ -137,14 +138,14 @@ class UserPasswordResetTest extends WebTestBase {
$timeout = $this->config('user.settings')->get('password_reset_timeout');
$bogus_timestamp = REQUEST_TIME - $timeout - 60;
$_uid = $this->account->id();
$this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account->getPassword(), $bogus_timestamp, $this->account->getLastLoginTime()));
$this->drupalGet("user/reset/$_uid/$bogus_timestamp/" . user_pass_rehash($this->account->getPassword(), $bogus_timestamp, $this->account->getLastLoginTime(), $this->account->id()));
$this->assertText(t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'), 'Expired password reset request rejected.');
// Create a user, block the account, and verify that a login link is denied.
$timestamp = REQUEST_TIME - 1;
$blocked_account = $this->drupalCreateUser()->block();
$blocked_account->save();
$this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account->getPassword(), $timestamp, $blocked_account->getLastLoginTime()));
$this->drupalGet("user/reset/" . $blocked_account->id() . "/$timestamp/" . user_pass_rehash($blocked_account->getPassword(), $timestamp, $blocked_account->getLastLoginTime(), $this->account->id()));
$this->assertResponse(403);
}
@ -177,4 +178,44 @@ class UserPasswordResetTest extends WebTestBase {
$this->drupalGet('user/password', array('query' => array('name' => $edit['name'])));
$this->assertFieldByName('name', $edit['name'], 'User name found.');
}
/**
* Make sure that users cannot forge password reset URLs of other users.
*/
function testResetImpersonation() {
// Create two identical user accounts except for the user name. They must
// have the same empty password, so we can't use $this->drupalCreateUser().
$edit = array();
$edit['name'] = $this->randomMachineName();
$edit['mail'] = $edit['name'] . '@example.com';
$edit['status'] = 1;
$user1 = User::create($edit);
$user1->save();
$edit['name'] = $this->randomMachineName();
$user2 = User::create($edit);
$user2->save();
// Unique password hashes are automatically generated, the only way to
// change that is to update it directly in the database.
db_update('users_field_data')
->fields(['pass' => NULL])
->condition('uid', [$user1->id(), $user2->id()], 'IN')
->execute();
\Drupal::entityManager()->getStorage('user')->resetCache();
$user1 = User::load($user1->id());
$user2 = User::load($user2->id());
$this->assertEqual($user1->getPassword(), $user2->getPassword(), 'Both users have the same password hash.');
// The password reset URL must not be valid for the second user when only
// the user ID is changed in the URL.
$reset_url = user_pass_reset_url($user1);
$attack_reset_url = str_replace("user/reset/{$user1->id()}", "user/reset/{$user2->id()}", $reset_url);
$this->drupalGet($attack_reset_url);
$this->assertNoText($user2->getUsername(), 'The invalid password reset page does not show the user name.');
$this->assertUrl('user/password', array(), 'The user is redirected to the password reset request page.');
$this->assertText('You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.');
}
}

12
core/modules/user/user.module
View File

@ -635,7 +635,7 @@ function user_pass_reset_url($account, $options = array()) {
array(
'uid' => $account->id(),
'timestamp' => $timestamp,
'hash' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime()),
'hash' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id()),
),
array(
'absolute' => TRUE,
@ -672,7 +672,7 @@ function user_cancel_url($account, $options = array()) {
return \Drupal::url('user.cancel_confirm', [
'user' => $account->id(),
'timestamp' => $timestamp,
'hashed_pass' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime())
'hashed_pass' => user_pass_rehash($account->getPassword(), $timestamp, $account->getLastLoginTime(), $account->id())
], $url_options);
}
@ -694,12 +694,14 @@ function user_cancel_url($account, $options = array()) {
* A UNIX timestamp, typically REQUEST_TIME.
* @param int $login
* The UNIX timestamp of the user's last login.
* @param int $uid
* The user ID.
*
* @return
* @return string
* A string that is safe for use in URLs and SQL statements.
*/
function user_pass_rehash($password, $timestamp, $login) {
return Crypt::hmacBase64($timestamp . $login, Settings::getHashSalt() . $password);
function user_pass_rehash($password, $timestamp, $login, $uid) {
return Crypt::hmacBase64($timestamp . $login . $uid, Settings::getHashSalt() . $password);
}
/**