bug #46249 [HttpFoundation] [Session] Regenerate invalid session id (peter17)

nicolas-grekas · nicolas-grekas · commit b37fc1effbfb · 2022-05-17T14:15:53.000+02:00
This PR was merged into the 4.4 branch. Discussion ---------- [HttpFoundation] [Session] Regenerate invalid session id | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #45755 | License | MIT | Doc PR | no Currently, having a PHPSESSID which does not match `/^[a-zA-Z0-9,\-]{1,123}$/` (see https://www.php.net/manual/fr/function.session-start.php) will produce a php.WARNING and then a RuntimeException (please read #45755). I don't think there is a nice way to handle this so I propose to simply ignore invalid values. With this PR, a value for PHPSESSID that does not match the regex will be ignored and a new session id will be generated. Then, the behavior will be the same as if no session existed, so a new session will be started and a new PHPSESSID will be defined. It looks like Session storage is currently untested so I don't know how to test this... Best regards Commits ------- d8f84c7 [HttpFoundation] [Session] Overwrite invalid session id
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
@@ -152,6 +152,12 @@ public function start()
             throw new \RuntimeException(sprintf('Failed to start the session because headers have already been sent by "%s" at line %d.', $file, $line));
         }
 
+        $sessionId = $_COOKIE[session_name()] ?? null;
+        if ($sessionId && !preg_match('/^[a-zA-Z0-9,-]{22,}$/', $sessionId)) {
+            // the session ID in the header is invalid, create a new one
+            session_id(session_create_id());
+        }
+
         // ok to try and start the session
         if (!session_start()) {
             throw new \RuntimeException('Failed to start the session.');
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php
@@ -293,4 +293,13 @@ public function testGetBagsOnceSessionStartedIsIgnored()
 
         $this->assertEquals($storage->getBag('flashes'), $bag);
     }
+
+    public function testRegenerateInvalidSessionId()
+    {
+        $_COOKIE[session_name()] = '&~[';
+        $started = (new NativeSessionStorage())->start();
+
+        $this->assertTrue($started);
+        $this->assertMatchesRegularExpression('/^[a-zA-Z0-9,-]{22,}$/', session_id());
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -293,4 +293,13 @@ public function testGetBagsOnceSessionStartedIsIgnored()`
`293`	`293`
`294`	`294`	`$this->assertEquals($storage->getBag('flashes'), $bag);`
`295`	`295`	`}`
	`296`	`+`
	`297`	`+ public function testRegenerateInvalidSessionId()`
	`298`	`+ {`
	`299`	`+ $_COOKIE[session_name()] = '&~[';`
	`300`	`+ $started = (new NativeSessionStorage())->start();`
	`301`	`+`
	`302`	`+ $this->assertTrue($started);`
	`303`	`+ $this->assertMatchesRegularExpression('/^[a-zA-Z0-9,-]{22,}$/', session_id());`
	`304`	`+ }`
`296`	`305`	`}`