-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k

Description
Symfony version(s) affected
6.1.3
Description
Hi, the error is similar to #46777, except that now the issue comes from another reason. Or maybe I misunderstood how Symfony sessions work...
Look at this condition:
symfony/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
Line 169 in 818d4dd
if ($sessionId && $this->saveHandler instanceof AbstractProxy && 'files' === $this->saveHandler->getSaveHandlerName() && !preg_match('/^[a-zA-Z0-9,-]{22,250}$/', $sessionId)) { |
When i'm using NativeSessionStorage
class, $this->saveHandler->getSaveHandlerName()
returns user
but should return files
. Therefore, the whole condition is never true
and the following warning persists:
[25-Jun-2022 20:54:15 UTC] PHP Warning: SessionHandler::read(): Session ID is too long or contains illegal characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in C:\app\vendor\symfony\http-foundation\Session\Storage\Handler\StrictSessionHandler.php on line 45
How to reproduce
use Symfony\Component\HttpFoundation\Session\Session;
require __DIR__ . '/vendor/autoload.php';
$_COOKIE[session_name()] = str_repeat('A', 230) . '.:%*$!@`'; // Bad session ID provided by a malicious user.
$session = new Session(); // Using `Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage` storage class.
$session->start();
Possible Solution
Below, the code defines $this->saveHandlerName
to \ini_get('session.save_handler')
when $handler instanceof \SessionHandler
.
symfony/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php
Lines 24 to 25 in 818d4dd
$this->wrapper = $handler instanceof \SessionHandler; | |
$this->saveHandlerName = $this->wrapper ? \ini_get('session.save_handler') : 'user'; |
But here, the code instantiates new StrictSessionHandler(new \SessionHandler())
that is not an instance of \SessionHandler
.
symfony/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
Lines 422 to 424 in 818d4dd
} elseif (!$saveHandler instanceof AbstractProxy) { | |
$saveHandler = new SessionHandlerProxy(new StrictSessionHandler(new \SessionHandler())); | |
} |
So, I'm really not sure, but a possible solution could be as below:
- $this->wrapper = $handler instanceof \SessionHandler;
+ $this->wrapper = $handler instanceof \SessionHandlerInterface;
Because both StrictSessionHandler
and \SessionHandler
implements \SessionHandlerInterface
.
Additional Context
No response