Generated with the help of an LLM.
OpenAI Codex used during the research of a security testing report find.
Problem/Motivation
UserController::getResetPassForm() does not verify the hash as it is done by the UserController::resetPass() by running UserController::determineErrorRedirect() that in turn runs OneTimeAuthentication::verifyHmac().
Steps to reproduce
- Obtain a valid reset link for account A.
- Request it with redirect following disabled, saving the session cookie
- Before requesting the redirect destination, change its UID to that of active account B. Send that request with the saved cookie.
- Observe that the form displays B’s username and indicates whether B has logged in before. Submitting the form does not log in as B; the login action verifies the hash again.
cURL commands that demonstrate the flow:
curl -sS -D - -o /dev/null -c /tmp/reset-cookies 'https://drupal.ddev.site/user/reset/3/1793973007/LWgjWN-4jwmA0I1co06uuBSOCSbPF3K2q2CqPmN3w3w'
curl -sS -b /tmp/reset-cookies 'https://drupal.ddev.site/user/reset/2'
Proposed resolution
Add hash verification to the UserController::getResetPassForm() right before the expiration date variable is set.
// Time out, in seconds, until login URL expires.
$timeout = $this->config('user.settings')->get('password_reset_timeout');
if (!$this->oneTimeAuthentication->verifyHmac($user, $timestamp, $hash, $timeout)) {
// Hash verification failed. Possiblt becase user ID did not match.
throw new AccessDeniedHttpException();
}
$expiration_date = $user->getLastLoginTime() ? $this->dateFormatter->format($timestamp + $timeout) : NULL;
Remaining tasks
Verify or improve the suggested fix.
User interface changes
None.
Introduced terminology
None.
API changes
None.
Data model changes
None.
Release notes snippet
Check hash to verify validity of passed user identifier.
Issue fork drupal-3635848
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
pjotr.savitski commentedAdded cURL commands that demonstrate the flow.
Comment #3
cilefen commentedComment #4
pjotr.savitski commentedComment #6
pjotr.savitski commentedAdded a merge request that has failures in automated checks due to unrelated tests failing.
Comment #7
pjotr.savitski commented