Skip to content

Commit ded1e07

Browse files
Merge branch '5.4' into 6.4
* 5.4: [HttpClient] Resolve hostnames in NoPrivateNetworkHttpClient [security-http] Check owner of persisted remember-me cookie
2 parents cc13b60 + cde02b0 commit ded1e07

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
@@ -100,9 +100,16 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
100100
throw new AuthenticationException('The cookie is incorrectly formatted.');
101101
}
102102

103-
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue());
103+
[$series, $tokenValue] = explode(':', $rememberMeDetails->getValue(), 2);
104104
$persistentToken = $this->tokenProvider->loadTokenBySeries($series);
105105

106+
if ($persistentToken->getUserIdentifier() !== $rememberMeDetails->getUserIdentifier() || $persistentToken->getClass() !== $rememberMeDetails->getUserFqcn()) {
107+
throw new AuthenticationException('The cookie\'s hash is invalid.');
108+
}
109+
110+
// content of $rememberMeDetails is not trustable. this prevents use of this class
111+
unset($rememberMeDetails);
112+
106113
if ($this->tokenVerifier) {
107114
$isTokenValid = $this->tokenVerifier->verifyToken($persistentToken, $tokenValue);
108115
} else {
@@ -112,11 +119,17 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
112119
throw new CookieTheftException('This token was already used. The account is possibly compromised.');
113120
}
114121

115-
if ($persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'] < time()) {
122+
$expires = $persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'];
123+
if ($expires < time()) {
116124
throw new AuthenticationException('The cookie has expired.');
117125
}
118126

119-
return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
127+
return parent::consumeRememberMeCookie(new RememberMeDetails(
128+
$persistentToken->getClass(),
129+
$persistentToken->getUserIdentifier(),
130+
$expires,
131+
$persistentToken->getLastUsed()->getTimestamp().':'.$series.':'.$tokenValue.':'.$persistentToken->getClass()
132+
));
120133
}
121134

122135
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
@@ -79,7 +79,7 @@ public function testConsumeRememberMeCookieValid()
7979
$this->tokenProvider->expects($this->any())
8080
->method('loadTokenBySeries')
8181
->with('series1')
82-
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTimeImmutable('-10 min')))
82+
->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', $lastUsed = new \DateTimeImmutable('-10 min')))
8383
;
8484

8585
$this->tokenProvider->expects($this->once())->method('updateToken')->with('series1');
@@ -97,11 +97,41 @@ public function testConsumeRememberMeCookieValid()
9797

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

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

0 commit comments

Comments
 (0)