Skip to content

Commit cde02b0

Browse files
committed
[security-http] Check owner of persisted remember-me cookie
1 parent 7152f0e commit cde02b0

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

RememberMe/PersistentRememberMeHandler.php

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,16 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
6666
throw new AuthenticationException('The cookie is incorrectly formatted.');
6767
}
6868

69-
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue());
69+
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue(), 2);
7070
$persistentToken = $this->tokenProvider->loadTokenBySeries($series);
7171

72+
if ($persistentToken->getUserIdentifier() !== $rememberMeDetails->getUserIdentifier() || $persistentToken->getClass() !== $rememberMeDetails->getUserFqcn()) {
73+
throw new AuthenticationException('The cookie\'s hash is invalid.');
74+
}
75+
76+
// content of $rememberMeDetails is not trustable. this prevents use of this class
77+
unset($rememberMeDetails);
78+
7279
if ($this->tokenVerifier) {
7380
$isTokenValid = $this->tokenVerifier->verifyToken($persistentToken, $tokenValue);
7481
} else {
@@ -78,11 +85,17 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
7885
throw new CookieTheftException('This token was already used. The account is possibly compromised.');
7986
}
8087

81-
if ($persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'] < time()) {
88+
$expires = $persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'];
89+
if ($expires < time()) {
8290
throw new AuthenticationException('The cookie has expired.');
8391
}
8492

85-
return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
93+
return parent::consumeRememberMeCookie(new RememberMeDetails(
94+
$persistentToken->getClass(),
95+
$persistentToken->getUserIdentifier(),
96+
$expires,
97+
$persistentToken->getLastUsed()->getTimestamp().':'.$series.':'.$tokenValue.':'.$persistentToken->getClass()
98+
));
8699
}
87100

88101
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void

Tests/RememberMe/PersistentRememberMeHandlerTest.php

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function testConsumeRememberMeCookieValid()
8080
$this->tokenProvider->expects($this->any())
8181
->method('loadTokenBySeries')
8282
->with('series1')
83-
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
83+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', $lastUsed = new \DateTime('-10 min')))
8484
;
8585

8686
$this->tokenProvider->expects($this->once())->method('updateToken')->with('series1');
@@ -98,11 +98,41 @@ public function testConsumeRememberMeCookieValid()
9898

9999
$this->assertSame($rememberParts[0], $cookieParts[0]); // class
100100
$this->assertSame($rememberParts[1], $cookieParts[1]); // identifier
101-
$this->assertSame($rememberParts[2], $cookieParts[2]); // expire
101+
$this->assertEqualsWithDelta($lastUsed->getTimestamp() + 31536000, (int) $cookieParts[2], 2); // expire
102102
$this->assertNotSame($rememberParts[3], $cookieParts[3]); // value
103103
$this->assertSame(explode(':', $rememberParts[3])[0], explode(':', $cookieParts[3])[0]); // series
104104
}
105105

106+
public function testConsumeRememberMeCookieInvalidOwner()
107+
{
108+
$this->tokenProvider->expects($this->any())
109+
->method('loadTokenBySeries')
110+
->with('series1')
111+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
112+
;
113+
114+
$rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'jeremy', 360, 'series1:tokenvalue');
115+
116+
$this->expectException(AuthenticationException::class);
117+
$this->expectExceptionMessage('The cookie\'s hash is invalid.');
118+
$this->handler->consumeRememberMeCookie($rememberMeDetails);
119+
}
120+
121+
public function testConsumeRememberMeCookieInvalidValue()
122+
{
123+
$this->tokenProvider->expects($this->any())
124+
->method('loadTokenBySeries')
125+
->with('series1')
126+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
127+
;
128+
129+
$rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'wouter', 360, 'series1:tokenvalue:somethingelse');
130+
131+
$this->expectException(AuthenticationException::class);
132+
$this->expectExceptionMessage('This token was already used. The account is possibly compromised.');
133+
$this->handler->consumeRememberMeCookie($rememberMeDetails);
134+
}
135+
106136
public function testConsumeRememberMeCookieValidByValidatorWithoutUpdate()
107137
{
108138
$verifier = $this->createMock(TokenVerifierInterface::class);

0 commit comments

Comments
 (0)