8000 [HttpFoundation] PHP Warning: `SessionHandler::read()`: Session ID is too long or contains illegal characters. · Issue #47126 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] PHP Warning: SessionHandler::read(): Session ID is too long or contains illegal characters. #47126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Rezyan opened this issue Jul 30, 2022 · 4 comments
Closed

[HttpFoundation] PHP Warning: SessionHandler::read(): Session ID is too long or contains illegal characters. #47126

Rezyan opened this issue Jul 30, 2022 · 4 comments
Labels
Bug HttpFoundation Status: Needs Review Status: Waiting feedback

Comments

@Rezyan
Copy link
Contributor
Rezyan commented Jul 30, 2022 •
edited
Loading

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
@Rezyan Rezyan added the Bug label Jul 30, 2022
@xabbuh
Copy link
Member
xabbuh commented Jul 30, 2022

Same as #46993?

@Rezyan
Copy link
Contributor Author
Rezyan commented Jul 30, 2022 •
edited
Loading

Same as #46993?

@xabbuh Yes, that's the same, sorry for duplicate! I think that my solution could work. What do you think about this?

@xabbuh
Copy link
Member
xabbuh commented Jul 30, 2022

@brokensourcecode I don't know. I think it's best if you open a PR with your idea to gather some feedback. I close here meanwhile as we still have the other issue.

@xabbuh xabbuh closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2022
@Rezyan Rezyan mentioned this issue Jul 30, 2022
Merged
@Rezyan
Copy link
Contributor Author
Rezyan commented Jul 30, 2022

@xabbuh The solution I suggested in my previous comments is meaningless, I thought wrong. I suggest another one here: #47130

nicolas-grekas added a commit that referenced this issue Aug 1, 2022
@nicolas-grekas
bug #47130 [HttpFoundation] Fix invalid ID not regenerated with nativ…
6c0f7da 
…e PHP file sessions (BrokenSourceCode)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] Fix invalid ID not regenerated with native PHP file sessions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #46993, #47126
| License       | MIT

Inside the [`SessionHandlerProxy`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php) class, the code defines `$this->saveHandlerName` to `\ini_get('session.save_handler')` when `$handler` is an instance of [`\SessionHandler`](https://www.php.net/manual/en/class.sessionhandler.php).
https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php#L24-L25

But inside the [`NativeSessionStorage`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php) class, the code create an instance of [`StrictSessionHandler`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/StrictSessionHandler.php) that doesn't inherit from [`\SessionHandler`](https://www.php.net/manual/en/class.sessionhandler.php) and is passed to the [`SessionHandlerProxy`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php) constructor.
https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php#L422-L424

Therefore, we could create a `isWrapper()` method inside the [`StrictSessionHandler`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/StrictSessionHandler.php) class to check if the wrapped handler is an internal PHP session handler ([`\SessionHandler`](https://www.php.net/manual/en/class.sessionhandler.php)), just like [`AbstractProxy::isWrapper()`](https://github.com/symfony/symfony/blob/818d4dda7de778726123bc6bd49488959d4186e7/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php#L50).

That's the only solution I have in mind right now.

Commits
-------

4775c88 [HttpFoundation] Fix invalid ID not regenerated with native PHP file sessions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment 47BF
Assignees
No one assigned
Labels
Bug HttpFoundation Status: Needs Review Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xabbuh @carsonbot @Rezyan
0