Skip to content
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.

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

SessionManager Validates session_id After session_start() Causing a Warning #3

Open
weierophinney opened this issue Dec 31, 2019 · 1 comment
Labels
Bug Something isn't working

Comments

@weierophinney
Copy link
Member

Hi,

session_start() in SessionManager is causing a warning using an id with invalid characters. This behaviour should be handled to prevent problems like "information disclosures". An attacker can trigger the warning too easy.

Code to reproduce the issue

        $config = new SessionConfig();
        $manager = Container::getDefaultManager();
        $manager->setConfig($config);

        $this->sessionContainer = new Container('foo', $manager);

Reproduce The Issue

% curl -I 'http://zend.local/' -H 'Cookie: PHPSESSID=_test_'
HTTP/2 500 
server: nginx/1.10.3
date: Tue, 02 Jul 2019 08:59:35 GMT
content-type: text/html; charset=UTF-8

Expected results

There a three possible ways to handle the situation:

  1. Suppress the warning, regenerate a new id, start the session again
if (! @session_start()) {
    $this->regenerateId();
    session_start();
}
  1. Suppress the warning but also throw an exception that the session is not be started
if (! @session_start()) {
    throw new Exception\RuntimeException('Failed to start the session');
}
  1. Just ignore the warning and move handling to the validators
@session_start();

I'm personally a fan of the first option because I think that the developer doesn't want to handle errors that occurred during session start.
"If the session doesn't start, just ignore the user provided id and create a new/correct one."

Actual results

.../vendor/zendframework/zend-session/src/SessionManager.php:140
session_start(): The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,' [E_WARNING]

Zend\Session\Validator\Id is trying to address the problem, but fails to do so because the validation happens after session_start().

Related Issue

I found #21, but @SvenRtbg's comment is not 100% addressing what I'm concerned about.
My concern is information disclosures when the system is not handling warnings properly (never show any errors to the end user).

I hope the filed issue is clear and has enough details. Let me know if you need more background.

Best regards


Originally posted by @sbani at zendframework/zend-session#119

@weierophinney weierophinney added the Bug Something isn't working label Dec 31, 2019
@weierophinney
Copy link
Member Author

Hi,

Yesterday i had a lot of errors (fuzzing attack on session)

error_reporting session_start(): Failed to write session lock: A BAD KEY WAS PROVIDED/CHARACTERS OUT OF RANGE
line 140
code 2
file /export/www/test/vendor/zendframework/zend-session/src/SessionManager.php

or

session_start(): error getting session from memcached: (0x7f4238e96300) A BAD KEY WAS PROVIDED/CHARACTERS OUT OF RANGE, Key provided had invalid character. -> libmemcached/key.cc:103

and this is also, because SessionManager validates session_id after session_start()

I checked @sbani code to reproduce this issue, and I can see that even though session_id does not validate, the session was created on the memcache side

[test@devel DEVEL]$ memcached-tool localhost:11211 dump
Dumping memcache contents
add test 0 1561244503 0


Originally posted by @digitalStyx at zendframework/zend-session#119 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant