Description
Symfony version(s) affected: 5.3.6
Description
This bug was originally raised at #28314 and (almost) fixed in #41175
The current implementation creates the cookie in this line
The problem with that line is that it creates a race condition between 2 concurrent requests (which #28314 was originally about):
So the chronology is:
- Browser emits 2 concurrent requests with identical remember-me cookie
- One request wins this race, authenticates a user with remember-me token, regenerates token, stores old in the cache, stores new in database, returns new token in the
Set-Cookie
- Now the second request is to be handled, because it's within 60 seconds and because the old value is still in the cache - it passes validation. But now we create a cookie in that line 100 and put the old value into it.
Now the browser looks fine, but it contains the remember-me token with not actual value, because the second request, which has lost the race - re-set the cookie value to the old one.
How to reproduce
- Authenticate with remember me
- Write down the remember me cookie somewhere
- Wait for at least a minute
- Delete session cookie
- Refresh the page - now you should reauthenticate with remember me
- Restore remember me cookie with the value from the step 2
- Wait for another minute and if you're using a cache with
appendOnly
- ensure you clear it. - Delete session cookie again
- Refresh the page
Now the session is lost and in the logs you can read This token was already used. The account is possibly compromised
.
Possible Solution
The cookie should only be returned if the new token value has been generated.
eg:
// if a token was regenerated less than a minute ago, there is no need to regenerate it
// if multiple concurrent requests reauthenticate a user we do not want to update the token several times
if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
$tokenLastUsed = new \DateTime();
if ($this->tokenVerifier) {
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
}
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue)); // moved inside condition body
}
Additional context
/cc @Seldaek