CVE-2023-46733: Session Fixation (CWE-384)
value: Symfony is a PHP framework for web and console applications and a set of reusable PHP components. Starting in versions 5.4.21 and 6.2.7 and prior to versions 5.4.31 and 6.3.8, `SessionStrategyListener` does not migrate the session after every successful login. It does so only in case the logged in user changes by means of checking the user identifier. In some use cases, the user identifier doesn't change between the verification phase and the successful login, while the token itself changes from one type (partially-authenticated) to another (fully-authenticated). When this happens, the session id should be regenerated to prevent possible session fixations, which is not the case at the moment. As of versions 5.4.31 and 6.3.8, Symfony now checks the type of the token in addition to the user identifier before deciding whether the session id should be regenerated.
CWE description: Authenticating a user, or otherwise establishing a new user session, without invalidating any existing session identifier gives an attacker the opportunity to steal authenticated sessions.
CVSS v3 base score: 6.5
Published: 2023-11-10T18:15Z
Repository: https://github.com/symfony/symfony
Language: PHP
Files changed: src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php, src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php
Commit date: 2023-11-03 17:13:08 +0100
Commit message: [Security] Fix possible session fixation when only the *token* changes

Diff: diff --git a/src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php b/src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php
index 311a52ffd98b..c6fcba88e3dc 100644
--- a/src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php
+++ b/src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php
@@ -46,11 +46,11 @@ class SessionStrategyListener implements EventSubscriberInterface
         if ($previousToken = $event->getPreviousToken()) {
             // @deprecated since Symfony 5.3, change to $token->getUserIdentifier() in 6.0
             $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
             $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
 
-            if ('' !== ($user ?? '') && $user === $previousUser) {
+            if ('' !== ($user ?? '') && $user === $previousUser && \get_class($token) === \get_class($previousToken)) {
                 return;
             }
         }
 
         $this->sessionAuthenticationStrategy->onAuthentication($request, $token);
diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php
index 51b8dc1878ed..29ef9b68c182 100644
--- a/src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/EventListener/SessionStrategyListenerTest.php
@@ -13,10 +13,11 @@ namespace Symfony\Component\Security\Http\Tests\EventListener;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Security\Core\Authentication\Token\NullToken;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\User\InMemoryUser;
 use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
 use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
@@ -79,10 +80,30 @@ class SessionStrategyListenerTest extends TestCase
         $event = new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function () {})), $token, $this->request, null, 'main_firewall', $previousToken);
 
         $this->listener->onSuccessfulLogin($event);
     }
 
+    public function testRequestWithSamePreviousUserButDifferentTokenType()
+    {
+        $this->configurePreviousSession();
+
+        $token = $this->createMock(NullToken::class);
+        $token->expects($this->once())
+            ->method('getUserIdentifier')
+            ->willReturn('test');
+        $previousToken = $this->createMock(UsernamePasswordToken::class);
+        $previousToken->expects($this->once())
+            ->method('getUserIdentifier')
+            ->willReturn('test');
+
+        $this->sessionAuthenticationStrategy->expects($this->once())->method('onAuthentication')->with($this->request, $token);
+
+        $event = new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function () {})), $token, $this->request, null, 'main_firewall', $previousToken);
+
+        $this->listener->onSuccessfulLogin($event);
+    }
+
     private function createEvent($firewallName)
     {
         return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function ($username) { return new InMemoryUser($username, null); })), $this->token, $this->request, null, $firewallName);
     }
 

Vulnerable code:             if ('' !== ($user ?? '') && $user === $previousUser) {
Fixed code:             if ('' !== ($user ?? '') && $user === $previousUser && \get_class($token) === \get_class($previousToken)) {
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
    public function testRequestWithSamePreviousUserButDifferentTokenType()
    {
        $this->configurePreviousSession();

        $token = $this->createMock(NullToken::class);
        $token->expects($this->once())
            ->method('getUserIdentifier')
            ->willReturn('test');
        $previousToken = $this->createMock(UsernamePasswordToken::class);
        $previousToken->expects($this->once())
            ->method('getUserIdentifier')
            ->willReturn('test');

        $this->sessionAuthenticationStrategy->expects($this->once())->method('onAuthentication')->with($this->request, $token);

        $event = new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function () {})), $token, $this->request, null, 'main_firewall', $previousToken);

        $this->listener->onSuccessfulLogin($event);
    }