Skip to content

Commit

Permalink
bug #35239 [Security\Http] Prevent canceled remember-me cookie from b…
Browse files Browse the repository at this point in the history
…eing accepted (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security\Http] Prevent canceled remember-me cookie from being accepted

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35198
| License       | MIT
| Doc PR        | -

`RememberMeServices::autoLogin()` only checks that the cookie exists in `$request->cookies` while `loginFail()` only alter `$request->attributes` (which allows child implementations to read the canceled cookie for e.g. removing a persistent one).
This makes `autoLogin()` checks for `request->attributes` first, which fixes the linked issue.

Failure expected on deps=high build.

Commits
-------

9b711b8 [Security] Prevent canceled remember-me cookie from being accepted
  • Loading branch information
nicolas-grekas committed Jan 8, 2020
2 parents 2f38a5a + 9b711b8 commit fd19bd7
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 2 deletions.
Expand Up @@ -33,7 +33,7 @@ public function testUserChangeClearsCookie()
$this->assertNotNull($cookieJar->get('REMEMBERME'));

$client->request('GET', '/foo');
$this->assertSame(200, $client->getResponse()->getStatusCode());
$this->assertRedirect($client->getResponse(), '/login');
$this->assertNull($cookieJar->get('REMEMBERME'));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Expand Up @@ -19,7 +19,7 @@
"php": "^5.5.9|>=7.0.8",
"ext-xml": "*",
"symfony/config": "~3.4|~4.0",
"symfony/security": "~3.4.36|~4.3.9|^4.4.1",
"symfony/security": "~3.4.37|~4.3.10|^4.4.3",
"symfony/dependency-injection": "^3.4.3|^4.0.3",
"symfony/http-kernel": "~3.4|~4.0",
"symfony/polyfill-php70": "~1.0"
Expand Down
Expand Up @@ -99,6 +99,10 @@ public function getSecret()
*/
final public function autoLogin(Request $request)
{
if (($cookie = $request->attributes->get(self::COOKIE_ATTR_NAME)) && null === $cookie->getValue()) {
return null;
}

if (null === $cookie = $request->cookies->get($this->options['name'])) {
return null;
}
Expand Down
Expand Up @@ -39,6 +39,17 @@ public function testAutoLoginReturnsNullWhenNoCookie()
$this->assertNull($service->autoLogin(new Request()));
}

public function testAutoLoginReturnsNullAfterLoginFail()
{
$service = $this->getService(null, ['name' => 'foo', 'path' => null, 'domain' => null]);

$request = new Request();
$request->cookies->set('foo', 'foo');

$service->loginFail($request);
$this->assertNull($service->autoLogin($request));
}

/**
* @group legacy
*/
Expand Down

0 comments on commit fd19bd7

Please sign in to comment.