8000 Refreshed RememberMe cookie contains invalid data · Issue #41891 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Refreshed RememberMe cookie contains invalid data #41891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
qurben opened this issue Jun 28, 2021 · 5 comments
Closed

Refreshed RememberMe cookie contains invalid data #41891

qurben opened this issue Jun 28, 2021 · 5 comments
Labels
Bug Security Status: Needs Review

Comments

@qurben
Copy link
Contributor
qurben commented Jun 28, 2021 •
edited
Loading

Symfony version(s) affected: 5.3.2 (with symfony/security-http 5.3.x-dev 81c183fd1527a2d09bd3b5c69bca3fc24ce18527 for another fix in rememberme)

Description

The rememberme cookie returned when a rememberme session is renewed is not properly formatted. This causes it to be rejected when it is used again.

How to reproduce

  1. Given a setup with rememberme enabled
  2. Login with remember me
  3. Remove session cookie
  4. Refresh
  5. Remove session cookie again
  6. There is no new session

Possible Solution

I pulled two cookies from my installation (a development version, see Additional context), the first one is the original REMEMBERME cookie and the second one is the one received after refreshing. In the first one the last value is base64 encoded and in the second one it isn't.

(originally I had these two mixed up)

Additional context

REMEMBERME=Q3NyRGVsZnRcZW50aXR5XHNlY3VyaXR5XEFjY291bnQ6TVRNME5RPT06MTYyNjEyNjUzOTpVUzNCS2lhNUpsV2hpT2hPSmZ2UWExWGxSNmRNMWJtR2NHRThuZzhQalRsYWFuMDNhM0ZISU9hOXVVYmo5VkVlbWptemVMTlU1cXA4SGVLSFYxVng0UT09Ojc0OWRjN2VkZTU3MzFhODRiZmVmMzIxYzQyZjgxNDI0ZDY2MDAzNTRkNTljMzI2MDQyNTA1ZGY2OTA4NTUyMTA%3D
url & base64 decoded: CsrDelft\entity\security\Account:MTM0NQ==:1626126539:US3BKia5JlWhiOhOJfvQa1XlR6dM1bmGcGE8ng8PjTlaan03a3FHIOa9uUbj9VEemjmzeLNU5qp8HeKHV1Vx4Q==:749dc7ede5731a84bfef321c42f81424d6600354d59c326042505df690855210

REMEMBERME=Q3NyRGVsZnRcZW50aXR5XHNlY3VyaXR5XEFjY291bnQ6TVRNME5RPT06MTYyNjEyNjUzOTpVUzNCS2lhNUpsV2hpT2hPSmZ2UWExWGxSNmRNMWJtR2NHRThuZzhQalRsYWFuMDNhM0ZISU9hOXVVYmo5VkVlbWptemVMTlU1cXA4SGVLSFYxVng0UT09OnFVWHRIVWJHR01QQXBwbVdmSkFGWjRlS1QyRElOY0VLRHhvaDFHM2JmVUxtMmhGSExDdVcwT1dGd0FrcExUdlloTzFLaGora2dadmltQ01XL0xPYzB3PT0%3D
url & base64 decoded: CsrDelft\entity\security\Account:MTM0NQ==:1626126539:US3BKia5JlWhiOhOJfvQa1XlR6dM1bmGcGE8ng8PjTlaan03a3FHIOa9uUbj9VEemjmzeLNU5qp8HeKHV1Vx4Q==:qUXtHUbGGMPAppmWfJAFZ4eKT2DINcEKDxoh1G3bfULm2hFHLCuW0OWFwAkpLTvYhO1Khj+kgZvimCMW/LOc0w==
@qurben qurben added the Bug label Jun 28, 2021
@jderusse
Copy link
Member

Could you check if #41741 fixes your issue?

@qurben
Copy link
Contributor Author
qurben commented Jun 29, 2021

With #41741 this issue is still there.

@qurben
Copy link
Contributor Author
qurben commented Jun 29, 2021 •
edited
Loading

It seems that in \Symfony\Component\Security\Http\RememberMe\PersistentRememberMeHandler::processRememberMe the $tokenValue variable is normally not base64 encoded, but when the timestamp block is triggered it is. But it might be that it should always be base64 encoded, but that this goes wrong somewhere else.

(or this might not be relevant at all)

@qurben
Copy link
Contributor Author
qurben commented Jun 29, 2021

I think I found the culprit, in PersistentRememberMeHandler the tokenValue is set on line 92, but this should be the hashed version of the tokenValue as is done in the createRememberMeCookie method. This creates an invalid cookie.

symfony/src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php

Lines 91 to 101 in 1eb5b35

if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
$tokenValue = base64_encode(random_bytes(64));
$tokenValueHash = $this->generateHash($tokenValue);
$tokenLastUsed = new \DateTime();
if ($this->tokenVerifier) {
$this->tokenVerifier->updateExistingToken($persistentToken, $tokenValueHash, $tokenLastUsed);
}
$this->tokenProvider->updateToken($series, $tokenValueHash, $tokenLastUsed);
}
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));

symfony/src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php

Lines 56 to 58 in 1eb5b35

$series = base64_encode(random_bytes(64));
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
$token = new PersistentToken(\get_class($user), method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), $series, $tokenValue, new \DateTime());

@qurben
Copy link
Contributor Author
qurben commented Jun 29, 2021

I can provide a patch for this.

@qurben qurben mentioned this issue Jun 29, 2021
Merged
@chalasr chalasr mentioned this issue Jul 16, 2021
Closed
chalasr pushed a commit that referenced this issue Jul 16, 2021
@qurben
[Security] fix #41891 Save hashed tokenValue in RememberMe cookie
9ccaa93
chalasr added a commit that referenced this issue Jul 16, 2021
@chalasr
bug #41897 [Security] fix #41891 Save hashed tokenValue in RememberMe…
643e29a 
… cookie (qurben)

This PR was merged into the 5.3 branch.

Discussion
----------

[Security] fix #41891 Save hashed tokenValue in RememberMe cookie

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #41891 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

The hashed tokenValue is expected in the RememberMe cookie. This was not the case when this branch was executed.
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch 5.x.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
-->

Commits
-------

9ccaa93 [Security] fix #41891 Save hashed tokenValue in RememberMe cookie
@chalasr chalasr closed this as completed Jul 16, 2021
@fabpot fabpot mentioned this issue Jul 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jderusse @qurben @xabbuh @chalasr @carsonbot
0