Merge branch '5.4' into 6.4

* 5.4: [HttpClient] Resolve hostnames in NoPrivateNetworkHttpClient [security-http] Check owner of persisted remember-me cookie
symfony · Nov 13, 2024 · ded1e07 · ded1e07
2 parents cc13b60 + cde02b0
commit ded1e07
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 5 deletions.
diff --git a/RememberMe/PersistentRememberMeHandler.php b/RememberMe/PersistentRememberMeHandler.php
@@ -100,9 +100,16 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
             throw new AuthenticationException('The cookie is incorrectly formatted.');
         }
 
-        [$series, $tokenValue] = explode(':', $rememberMeDetails->getValue());
+        [$series, $tokenValue] = explode(':', $rememberMeDetails->getValue(), 2);
         $persistentToken = $this->tokenProvider->loadTokenBySeries($series);
 
+        if ($persistentToken->getUserIdentifier() !== $rememberMeDetails->getUserIdentifier() || $persistentToken->getClass() !== $rememberMeDetails->getUserFqcn()) {
+            throw new AuthenticationException('The cookie\'s hash is invalid.');
+        }
+
+        // content of $rememberMeDetails is not trustable. this prevents use of this class
+        unset($rememberMeDetails);
+
         if ($this->tokenVerifier) {
             $isTokenValid = $this->tokenVerifier->verifyToken($persistentToken, $tokenValue);
         } else {
@@ -112,11 +119,17 @@ public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): U
             throw new CookieTheftException('This token was already used. The account is possibly compromised.');
         }
 
-        if ($persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'] < time()) {
+        $expires = $persistentToken->getLastUsed()->getTimestamp() + $this->options['lifetime'];
+        if ($expires < time()) {
             throw new AuthenticationException('The cookie has expired.');
         }
 
-        return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
+        return parent::consumeRememberMeCookie(new RememberMeDetails(
+            $persistentToken->getClass(),
+            $persistentToken->getUserIdentifier(),
+            $expires,
+            $persistentToken->getLastUsed()->getTimestamp().':'.$series.':'.$tokenValue.':'.$persistentToken->getClass()
+        ));
     }
 
     public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void

diff --git a/Tests/RememberMe/PersistentRememberMeHandlerTest.php b/Tests/RememberMe/PersistentRememberMeHandlerTest.php
@@ -79,7 +79,7 @@ public function testConsumeRememberMeCookieValid()
         $this->tokenProvider->expects($this->any())
             ->method('loadTokenBySeries')
             ->with('series1')
-            ->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTimeImmutable('-10 min')))
+            ->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', $lastUsed = new \DateTimeImmutable('-10 min')))
         ;
 
         $this->tokenProvider->expects($this->once())->method('updateToken')->with('series1');
@@ -97,11 +97,41 @@ public function testConsumeRememberMeCookieValid()
 
         $this->assertSame($rememberParts[0], $cookieParts[0]); // class
         $this->assertSame($rememberParts[1], $cookieParts[1]); // identifier
-        $this->assertSame($rememberParts[2], $cookieParts[2]); // expire
+        $this->assertEqualsWithDelta($lastUsed->getTimestamp() + 31536000, (int) $cookieParts[2], 2); // expire
         $this->assertNotSame($rememberParts[3], $cookieParts[3]); // value
         $this->assertSame(explode(':', $rememberParts[3])[0], explode(':', $cookieParts[3])[0]); // series
     }
 
+    public function testConsumeRememberMeCookieInvalidOwner()
+    {
+        $this->tokenProvider->expects($this->any())
+            ->method('loadTokenBySeries')
+            ->with('series1')
+            ->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
+        ;
+
+        $rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'jeremy', 360, 'series1:tokenvalue');
+
+        $this->expectException(AuthenticationException::class);
+        $this->expectExceptionMessage('The cookie\'s hash is invalid.');
+        $this->handler->consumeRememberMeCookie($rememberMeDetails);
+    }
+
+    public function testConsumeRememberMeCookieInvalidValue()
+    {
+        $this->tokenProvider->expects($this->any())
+            ->method('loadTokenBySeries')
+            ->with('series1')
+            ->willReturn(new PersistentToken(InMemoryUser::class, 'wouter', 'series1', 'tokenvalue', new \DateTime('-10 min')))
+        ;
+
+        $rememberMeDetails = new RememberMeDetails(InMemoryUser::class, 'wouter', 360, 'series1:tokenvalue:somethingelse');
+
+        $this->expectException(AuthenticationException::class);
+        $this->expectExceptionMessage('This token was already used. The account is possibly compromised.');
+        $this->handler->consumeRememberMeCookie($rememberMeDetails);
+    }
+
     public function testConsumeRememberMeCookieValidByValidatorWithoutUpdate()
     {
         $verifier = $this->createMock(TokenVerifierInterface::class);