CVE-2022-24895: Session Fixation (CWE-384)
value: Symfony is a PHP framework for web and console applications and a set of reusable PHP components. When authenticating users Symfony by default regenerates the session ID upon login, but preserves the rest of session attributes. Because this does not clear CSRF tokens upon login, this might enables same-site attackers to bypass the CSRF protection mechanism by performing an attack similar to a session-fixation. This issue has been fixed in the 4.4 branch. 


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: 8.8
Published: 2023-02-03T22:15Z
Repository: https://github.com/symfony/symfony
Language: PHP
Files changed: src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml, src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php, src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php, src/Symfony/Bundle/SecurityBundle/composer.json, src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php, src/Symfony/Component/Security/Http/Tests/Session/SessionAuthenticationStrategyTest.php
Commit date: 2023-01-24 11:39:54 +0100
Commit message: [Security/Http] Remove CSRF tokens from storage on successful login

Diff: diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
index 3491383b8bba..eabe5e547fad 100644
--- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
+++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
@@ -61,10 +61,11 @@
             <argument>%security.authentication.trust_resolver.rememberme_class%</argument>
         </service>
 
         <service id="security.authentication.session_strategy" class="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy">
             <argument>%security.authentication.session_strategy.strategy%</argument>
+            <argument type="service" id="security.csrf.token_storage" on-invalid="ignore" />
         </service>
         <service id="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface" alias="security.authentication.session_strategy" />
 
         <service id="security.authentication.session_strategy_noop" class="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy" public="false">
             <argument>none</argument>
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php
index 1a672d70f833..08ea67a6416f 100644
--- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php
+++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php
@@ -17,16 +17,19 @@ class CsrfFormLoginTest extends AbstractWebTestCase
      * @dataProvider getConfigs
      */
     public function testFormLoginAndLogoutWithCsrfTokens($config)
     {
         $client = $this->createClient(['test_case' => 'CsrfFormLogin', 'root_config' => $config]);
+        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
 
         $form = $client->request('GET', '/login')->selectButton('login')->form();
         $form['user_login[username]'] = 'johannes';
         $form['user_login[password]'] = 'test';
         $client->submit($form);
 
+        $this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
+
         $this->assertRedirect($client->getResponse(), '/profile');
 
         $crawler = $client->followRedirect();
 
         $text = $crawler->text(null, true);
@@ -46,15 +49,18 @@ class CsrfFormLoginTest extends AbstractWebTestCase
      * @dataProvider getConfigs
      */
     public function testFormLoginWithInvalidCsrfToken($config)
     {
         $client = $this->createClient(['test_case' => 'CsrfFormLogin', 'root_config' => $config]);
+        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
 
         $form = $client->request('GET', '/login')->selectButton('login')->form();
         $form['user_login[_token]'] = '';
         $client->submit($form);
 
+        $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
+
         $this->assertRedirect($client->getResponse(), '/login');
 
         $text = $client->followRedirect()->text(null, true);
         $this->assertStringContainsString('Invalid CSRF token.', $text);
     }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php
index cb7868f3256e..465027f42f0c 100644
--- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php
+++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php
@@ -34,19 +34,17 @@ class LogoutTest extends AbstractWebTestCase
     }
 
     public function testCsrfTokensAreClearedOnLogout()
     {
         $client = $this->createClient(['test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml']);
-        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
 
         $client->request('POST', '/login', [
             '_username' => 'johannes',
             '_password' => 'test',
         ]);
 
-        $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
-        $this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo'));
+        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
 
         $client->request('GET', '/logout');
 
         $this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
     }
diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
index 1106acfa008c..4061646f399f 100644
--- a/src/Symfony/Bundle/SecurityBundle/composer.json
+++ b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -23,11 +23,11 @@
         "symfony/http-kernel": "^4.4",
         "symfony/polyfill-php80": "^1.16",
         "symfony/security-core": "^4.4",
         "symfony/security-csrf": "^4.2|^5.0",
         "symfony/security-guard": "^4.2|^5.0",
-        "symfony/security-http": "^4.4.5"
+        "symfony/security-http": "^4.4.50"
     },
     "require-dev": {
         "doctrine/annotations": "^1.10.4",
         "symfony/asset": "^3.4|^4.0|^5.0",
         "symfony/browser-kit": "^4.2|^5.0",
diff --git a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php
index a4bb88818d45..73691058d0ce 100644
--- a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php
+++ b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php
@@ -11,10 +11,11 @@
 
 namespace Symfony\Component\Security\Http\Session;
 
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;
 
 /**
  * The default session strategy implementation.
  *
  * Supports the following strategies:
@@ -29,14 +30,19 @@ class SessionAuthenticationStrategy implements SessionAuthenticationStrategyInte
     public const NONE = 'none';
     public const MIGRATE = 'migrate';
     public const INVALIDATE = 'invalidate';
 
     private $strategy;
+    private $csrfTokenStorage = null;
 
-    public function __construct(string $strategy)
+    public function __construct(string $strategy, ClearableTokenStorageInterface $csrfTokenStorage = null)
     {
         $this->strategy = $strategy;
+
+        if (self::MIGRATE === $strategy) {
+            $this->csrfTokenStorage = $csrfTokenStorage;
+        }
     }
 
     /**
      * {@inheritdoc}
      */
@@ -45,14 +51,16 @@ class SessionAuthenticationStrategy implements SessionAuthenticationStrategyInte
         switch ($this->strategy) {
             case self::NONE:
                 return;
 
             case self::MIGRATE:
-                // Note: this logic is duplicated in several authentication listeners
-                // until Symfony 5.0 due to a security fix with BC compat
                 $request->getSession()->migrate(true);
 
+                if ($this->csrfTokenStorage) {
+                    $this->csrfTokenStorage->clear();
+                }
+
                 return;
 
             case self::INVALIDATE:
                 $request->getSession()->invalidate();
 
diff --git a/src/Symfony/Component/Security/Http/Tests/Session/SessionAuthenticationStrategyTest.php b/src/Symfony/Component/Security/Http/Tests/Session/SessionAuthenticationStrategyTest.php
index 010bb0edf2cf..2051f3423914 100644
--- a/src/Symfony/Component/Security/Http/Tests/Session/SessionAuthenticationStrategyTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/Session/SessionAuthenticationStrategyTest.php
@@ -12,10 +12,11 @@
 namespace Symfony\Component\Security\Http\Tests\Session;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;
 use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
 use Symfony\Component\Security\Http\Tests\Fixtures\TokenInterface;
 
 class SessionAuthenticationStrategyTest extends TestCase
 {
@@ -55,10 +56,22 @@ class SessionAuthenticationStrategyTest extends TestCase
 
         $strategy = new SessionAuthenticationStrategy(SessionAuthenticationStrategy::INVALIDATE);
         $strategy->onAuthentication($this->getRequest($session), $this->createMock(TokenInterface::class));
     }
 
+    public function testCsrfTokensAreCleared()
+    {
+        $session = $this->createMock(SessionInterface::class);
+        $session->expects($this->once())->method('migrate')->with($this->equalTo(true));
+
+        $csrfStorage = $this->createMock(ClearableTokenStorageInterface::class);
+        $csrfStorage->expects($this->once())->method('clear');
+
+        $strategy = new SessionAuthenticationStrategy(SessionAuthenticationStrategy::MIGRATE, $csrfStorage);
+        $strategy->onAuthentication($this->getRequest($session), $this->createMock(TokenInterface::class));
+    }
+
     private function getRequest($session = null)
     {
         $request = $this->createMock(Request::class);
 
         if (null !== $session) {

Vulnerable code:         static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
        $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
        $this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo'));
        "symfony/security-http": "^4.4.5"
    public function __construct(string $strategy)
                // Note: this logic is duplicated in several authentication listeners
                // until Symfony 5.0 due to a security fix with BC compat
Fixed code:             <argument type="service" id="security.csrf.token_storage" on-invalid="ignore" />
        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
        $this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo'));

        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
        $this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));

        static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
        "symfony/security-http": "^4.4.50"
use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;
    private $csrfTokenStorage = null;
    public function __construct(string $strategy, ClearableTokenStorageInterface $csrfTokenStorage = null)

        if (self::MIGRATE === $strategy) {
            $this->csrfTokenStorage = $csrfTokenStorage;
        }
                if ($this->csrfTokenStorage) {
                    $this->csrfTokenStorage->clear();
                }

use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;
    public function testCsrfTokensAreCleared()
    {
        $session = $this->createMock(SessionInterface::class);
        $session->expects($this->once())->method('migrate')->with($this->equalTo(true));

        $csrfStorage = $this->createMock(ClearableTokenStorageInterface::class);
        $csrfStorage->expects($this->once())->method('clear');

        $strategy = new SessionAuthenticationStrategy(SessionAuthenticationStrategy::MIGRATE, $csrfStorage);
        $strategy->onAuthentication($this->getRequest($session), $this->createMock(TokenInterface::class));
    }