php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Doc Bug #74106 session_regenerate_id is misleading
Submitted: 2017-02-16 11:23 UTC Modified: 2017-02-16 20:16 UTC
Votes:2
Avg. Score:4.0 ± 1.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: php at pointpro dot nl Assigned: yohgaki (profile)
Status: Assigned Package: Session related
PHP Version: Irrelevant OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: php at pointpro dot nl
New email:
PHP Version: OS:

 

 [2017-02-16 11:23 UTC] php at pointpro dot nl 
Description:
------------
Please do correct me if I'm wrong, but it seems to me the documentation on session_regenerate_id is a bit dangerous and misleading.

It states that you should take additional precautions to avoid session loss, and the example basically gives the following pattern:

my_session_start
* start session
* if session ended in last 10 minutes, redirect to new session
* if session ended longer ago, remove authentication information

my_session_regenerate_id:
* create new session_id
* store new session_id in old session
* mark current session as destroyed
* start new session with new session_id


What my usual use case for using session_generate_id is that a user logged in or out. A new session ID is an additional precaution against session hijacking - if an attacker somehow manage to sneak in his own session id to his victim and convinces him to log in, his own session will be logged in because it's the same.

Given the pattern described in the documentation, this approach is void because even though a new session id is generated, it is stored in the old session. So if the attacker refreshes his page within 10 minutes after he convinces his victim to log in, he will be redirected to the new session, too, and still be logged in.

Unless I'm missing something here, I think the documentation is actually suggesting to introduce a session hijacking vulnerability to users.

While it is true that in bad network situations session loss may occur if the newly generated session ID never made it to the client, this risk does not seem worth the vulnerability introduced by work around, and at least there should be a big warning sign stating that this approach introduces other security risks.

This problem could be mitigated by at least adding a check for $_SERVER['REMOTE_ADDR'] and $_SERVER['HTTP_USER_AGENT'] before redirecting to the new session.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-02-16 11:34 UTC] requinix@php.net
-Package: Documentation problem +Package: Session related -Assigned To: +Assigned To: yohgaki
 [2017-02-16 19:50 UTC] yohgaki@php.net 
There should be window opened for "Legitimate Users". 
The example code open window for 5 minutes for unstable networks. e.g. Subway, Elevator, etc.

The window should be shorter as much as it is possible. However, shorter window could results in lost sessions.

The example code in the manual page has invalid (old session ID access) session detection also. Either "Legitimate User" or "Attacker" will be notified invalid access. (If your app is smart enough, you can notify both. i.e. When the exception is raised, get the other session (or even better, get all user's sessions for the user) and set "Invalid Access Flag", then notify.)

More discussions and issues are described in 
https://wiki.php.net/rfc/precise_session_management
https://wiki.php.net/rfc/session-use-strict-mode

Although above RFCs are declined, OWASP seems to add many issues and aspects to reflect above.
https://www.owasp.org/index.php/Session_Management_Cheat_Sheet

I was intended to improve session documents when above RFCs are accepted, but they didn't. session_regenerate_id() documentation (as well as other docs) could be improved, so I keep this bug open for the time being.

"While it is true that in bad network situations session loss may occur if the newly generated session ID never made it to the client, this risk does not seem worth the vulnerability introduced by work around, and at least there should be a big warning sign stating that this approach introduces other security risks."

I suppose you think "session_regenerate_id(true)" is better. While it is true by ignoring risks of lost sessions, it can happen more often. For example, when I tried regenerate_id(true) for every requests, this caused "Client side race condition" and results in lost sessions in every few thousands requests in 2015. Likeness of lost sessions depends on "Web browser" since this is client side race condition.

This approach "session_regenerate_id(true)" has other issue. "Session hijack attack detection" is not possible by "session_regenerate_id(true)". Above my approach that "Open a little window and detect attack" is better because "Legitimate users" could know possible attacks.
 [2017-02-16 20:01 UTC] yohgaki@php.net 
When session hijack is possible for whatever reasons, e.g. Malicious browser plugins that steal session ID/JavaScript injections/etc, chances are high that attackers are able to steal sessions as many as times attackers want.

"Open a little attack window and detect attack"(Best) provides much better security than "Leave old sessions behind"(The worst which is current session module default) or "Remove old session immediately"(Better, but far from optimal).
 [2017-02-16 20:16 UTC] yohgaki@php.net 
"For example, when I tried regenerate_id(true) for every requests, this caused "Client side race condition" and results in lost sessions in every few thousands requests in 2015. Likeness of lost sessions depends on "Web browser" since this is client side race condition."

This test was performed on "localhost"(127.0.0.1) which must be very stable network, BTW.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 12:01:29 2024 UTC