Issue #2803921 by roderik, mcdruid, aerozeppelin, David_Rothstein, drumm, pwolanin, locokiter, greggles, cilefen, Fabianx: A valid one-time login link may be leaked by the referer header to 3rd parties

merge-requests/471/head
mcdruid 2021-03-23 21:38:40 +00:00
parent 7ba88d9d4f
commit 41b03d753a
3 changed files with 177 additions and 12 deletions

View File

@ -80,3 +80,42 @@ function user_form_test_user_account_submit($form, &$form_state) {
// test for bugs that can be triggered in contributed modules.
$form_state['rebuild'] = TRUE;
}
/**
* Implements hook_form_FORM_ID_alter().
*/
function user_form_test_form_user_pass_reset_alter(&$form, &$form_state) {
// An unaltered form has no form values; the uid/timestmap/"confirm" are in
// the URL arguments. (If for some reason a form_alter method needs the
// hash, it can be retrieved from $form['#action'].)
if (!is_numeric(arg(2)) || !is_numeric(arg(3)) || !is_string(arg(4)) || arg(4) !== 'confirm') {
throw new Exception("Something unexpected changed in the user_pass_reset form; we are not getting the UID/timestamp/'confirm' passed anymore.");
}
// Use the variable system to communicate to the test code, since we don't
// share a session with it.
variable_set('user_test_pass_reset_form_build_' . arg(2), TRUE);
$form['#submit'][] = 'user_form_test_form_user_pass_reset_submit';
// We must cache the form to ensure the form builder (user_pass_reset()) is
// skipped when processing the submitted form, otherwise we get redirected
// already during form build.
$form_state['cache'] = TRUE;
}
/**
* Submit function for user_pass_reset().
*/
function user_form_test_form_user_pass_reset_submit($form, &$form_state) {
// On submit, the hash is in arg(4).
if (!is_numeric(arg(2)) || !is_numeric(arg(3)) || !is_string(arg(4)) || strlen(arg(4)) < 32) {
throw new Exception("Something unexpected changed in the user_pass_reset form; we are not getting the UID/timestamp/hash passed anymore.");
}
variable_set('user_test_pass_reset_form_submit_' . arg(2), TRUE);
// Because the form does no further processing and has no redirect set,
// drupal_redirect_form() will redirect back to ourselves
// (user/reset/UID/TIMESTAMP/HASH/login); we will be logged in and redirected
// while the form is built again. FYI: we cannot set $form_state['rebuild']
// to get around the first redirect without further hacks, because then the
// form won't pass the hash. (Our original $form_state['build_info']['args']
// contains "confirm" for the 3rd argument.)
}

View File

@ -134,10 +134,25 @@ function user_pass_submit($form, &$form_state) {
/**
* Menu callback; process one time login link and redirects to the user page on success.
*
* In order to never disclose password reset hashes via referrer headers or
* web browser history, this function always issues a redirect when a valid
* password reset hash is in the URL.
*/
function user_pass_reset($form, &$form_state, $uid, $timestamp, $hashed_pass, $action = NULL) {
global $user;
// Check for a reset hash in the session. Immediately remove it (to prevent it
// from being reused, for example if this page is visited again via the
// browser history) and store it for later use.
if (isset($_SESSION['pass_reset_hash'])) {
$session_reset_hash = $_SESSION['pass_reset_hash'];
unset($_SESSION['pass_reset_hash']);
}
else {
$session_reset_hash = NULL;
}
// When processing the one-time login link, we have to make sure that a user
// isn't already logged in.
if ($user->uid) {
@ -182,7 +197,36 @@ function user_pass_reset($form, &$form_state, $uid, $timestamp, $hashed_pass, $a
drupal_set_message(t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'), 'error');
drupal_goto('user/password');
}
elseif ($account->uid && $timestamp >= $account->login && $timestamp <= $current && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid)) {
// Validate the reset hash from the URL or from the session.
$is_valid = FALSE;
if ($account->uid && $timestamp >= $account->login && $timestamp <= $current) {
// If the reset hash in the URL is valid, put it in the session and
// redirect to the same URL but with the hash replaced by an invalid
// one ("confirm"). This prevents disclosing the hash via referrer
// headers or web browser history.
if ($hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid)) {
if ($action === 'login') {
// The 'login' action redirects directly to the user edit form.
$is_valid = TRUE;
}
else {
$_SESSION['pass_reset_hash'] = $hashed_pass;
$args = arg();
foreach ($args as &$arg) {
if ($arg == $hashed_pass) {
$arg = 'confirm';
}
}
$path = implode('/', $args);
drupal_goto($path, array('query' => drupal_get_query_parameters()));
}
}
// If the reset hash from the session is valid, use that.
elseif ($session_reset_hash && $session_reset_hash == user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid)) {
$is_valid = TRUE;
}
}
if ($is_valid) {
// First stage is a confirmation form, then login
if ($action == 'login') {
// Set the new user.
@ -204,7 +248,11 @@ function user_pass_reset($form, &$form_state, $uid, $timestamp, $hashed_pass, $a
$form['help'] = array('#markup' => '<p>' . t('This login can be used only once.') . '</p>');
$form['actions'] = array('#type' => 'actions');
$form['actions']['submit'] = array('#type' => 'submit', '#value' => t('Log in'));
$form['#action'] = url("user/reset/$uid/$timestamp/$hashed_pass/login");
$form['#action'] = url("user/reset/$uid/$timestamp/$session_reset_hash/login");
// Prevent the browser from storing this page so that the token will
// not be visible in the form action if the back button is used to
// revisit this page.
drupal_add_http_header('Cache-Control', 'no-store');
return $form;
}
}

View File

@ -480,6 +480,10 @@ class UserLoginTestCase extends DrupalWebTestCase {
class UserPasswordResetTestCase extends DrupalWebTestCase {
protected $profile = 'standard';
function setUp() {
parent::setUp('user_form_test');
}
public static function getInfo() {
return array(
'name' => 'Reset password',
@ -491,20 +495,38 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
/**
* Retrieves password reset email and extracts the login link.
*/
public function getResetURL() {
public function getResetURL($bypass_form = FALSE) {
// Assume the most recent email.
$_emails = $this->drupalGetMails();
$email = end($_emails);
$urls = array();
preg_match('#.+user/reset/.+#', $email['body'], $urls);
return $urls[0];
return $urls[0] . ($bypass_form ? '/login' : '');
}
/**
* Generates login link.
*/
public function generateResetURL($account, $bypass_form = FALSE) {
return user_pass_reset_url($account) . ($bypass_form ? '/login' : '');
}
/**
* Turns a password reset URL into a 'confirm' URL.
*/
public function getConfirmURL($reset_url) {
// Last part is always the hash; replace with "confirm".
$parts = explode('/', $reset_url);
array_pop($parts);
array_push($parts, 'confirm');
return implode('/', $parts);
}
/**
* Tests password reset functionality.
*/
function testUserPasswordReset() {
function testUserPasswordReset($use_direct_login_link = FALSE) {
// Create a user.
$account = $this->drupalCreateUser();
$this->drupalLogin($account);
@ -540,11 +562,19 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
);
field_create_instance($instance);
$resetURL = $this->getResetURL();
variable_del("user_test_pass_reset_form_submit_{$account->uid}");
$resetURL = $this->getResetURL($use_direct_login_link);
$this->drupalGet($resetURL);
// Check successful login.
$this->drupalPost(NULL, NULL, t('Log in'));
if (!$use_direct_login_link) {
$this->assertUrl($this->getConfirmURL($resetURL), array(), 'The user is redirected to the reset password confirm form.');
$this->drupalPost(NULL, NULL, t('Log in'));
// The form was fully processed before redirecting.
$form_submit_handled = variable_get("user_test_pass_reset_form_submit_{$account->uid}", FALSE);
$this->assertTrue($form_submit_handled, 'A custom submit handler executed.');
}
$this->assertText('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.');
// Make sure the Ajax request from uploading a file does not invalidate the
// reset token.
@ -559,6 +589,16 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
$edit = array('pass[pass1]' => $password, 'pass[pass2]' => $password);
$this->drupalPost(NULL, $edit, t('Save'));
$this->assertText(t('The changes have been saved.'), 'Forgotten password changed.');
// Ensure blocked and deleted accounts can't access the direct login link.
$this->drupalLogout();
$reset_url = $this->generateResetURL($account, $use_direct_login_link);
user_save($account, array('status' => 0));
$this->drupalGet($reset_url);
$this->assertResponse(403);
user_delete($account->uid);
$this->drupalGet($reset_url);
$this->assertResponse(403);
}
/**
@ -642,21 +682,51 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
/**
* Test user password reset while logged in.
*/
function testUserPasswordResetLoggedIn() {
function testUserPasswordResetLoggedIn($use_direct_login_link = FALSE) {
$another_account = $this->drupalCreateUser();
$account = $this->drupalCreateUser();
$this->drupalLogin($account);
// Make sure the test account has a valid password.
user_save($account, array('pass' => user_password()));
// Try to use the login link while logged in as a different user.
// Generate one time login link.
$reset_url = user_pass_reset_url($account);
$reset_url = $this->generateResetURL($another_account, $use_direct_login_link);
$this->drupalGet($reset_url);
$this->assertRaw(t(
'Another user (%other_user) is already logged into the site on this computer, but you tried to use a one-time link for user %resetting_user. Please <a href="!logout">logout</a> and try using the link again.',
array('%other_user' => $account->name, '%resetting_user' => $another_account->name, '!logout' => url('user/logout'))
));
$this->assertText('Reset password');
$this->drupalPost(NULL, NULL, t('Log in'));
// Test the link for a deleted user while logged in.
user_delete($another_account->uid);
$this->drupalGet($reset_url);
$this->assertText('The one-time login link you clicked is invalid.');
// Generate a one time login link for the logged-in user.
$fapi_action = $use_direct_login_link ? 'build' : 'submit';
variable_del("user_test_pass_reset_form_{$fapi_action}_{$account->uid}");
$reset_url = $this->generateResetURL($account, $use_direct_login_link);
$this->drupalGet($reset_url);
if ($use_direct_login_link) {
// The form is never fully built; user is logged out (session destroyed)
// and redirected to the same URL, then logged in again and redirected
// during form build.
$form_built = variable_get("user_test_pass_reset_form_build_{$account->uid}", FALSE);
$this->assertTrue(!$form_built, 'The password reset form was never fully built.');
}
else {
$this->assertUrl($this->getConfirmURL($reset_url), array(), 'The user is redirected to the reset password confirm form.');
$this->assertText('Reset password');
$this->drupalPost(NULL, NULL, t('Log in'));
// The form was fully processed before redirecting.
$form_submit_handled = variable_get("user_test_pass_reset_form_submit_{$account->uid}", FALSE);
$this->assertTrue($form_submit_handled, 'A custom submit handler executed.');
}
$this->assertText('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.');
// The user can change the forgotten password on the page they are
// redirected to.
$pass = user_password();
$edit = array(
'pass[pass1]' => $pass,
@ -667,6 +737,14 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
$this->assertText('The changes have been saved.');
}
/**
* Test direct login link that bypasses the password reset form.
*/
function testUserDirectLogin() {
$this->testUserPasswordReset(TRUE);
$this->testUserPasswordResetLoggedIn(TRUE);
}
/**
* Attempts login using an expired password reset link.
*/
@ -770,7 +848,7 @@ class UserPasswordResetTestCase extends DrupalWebTestCase {
$reset_url = url("user/reset/$user1->uid/$timestamp/$reset_url_token", array('absolute' => TRUE));
$this->drupalGet($reset_url);
$this->assertText($user1->name, 'The valid password reset page shows the user name.');
$this->assertUrl($reset_url, array(), 'The user remains on the password reset login page.');
$this->assertUrl($this->getConfirmURL($reset_url), array(), 'The user is redirected to the reset password confirm form.');
$this->assertNoText('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.');
}