Edit report at https://bugs.php.net/bug.php?id=65746&edit=1
ID: 65746
Updated by: yohgaki@php.net
Reported by: yohgaki@php.net
Summary: session_regenerate_id() should not delete old
session data immediately.
Status: Assigned
Type: Feature/Change Request
Package: Session related
Operating System: Any
PHP Version: Any
Assigned To: yohgaki
Block user comment: N
Private report: N
New Comment:
Your idea is against OWASP guideline, for example.
Timestamps must be managed and these timestamps must be used for precise session life cycle
management. Mandatory requirements are better to be managed as default feature.
Current implementation is simply broken because it removes old session data immediately or hopes
removal by GC.
Previous Comments:
------------------------------------------------------------------------
[2018-06-01 11:03:15] tony at marston-home dot demon dot co dot uk
I was referring to your post of [2016-10-17 06:38 UTC] in which you said
"session_regenerate_id() depreciation is a option. We shouldn't keep security related
broken function" which implied that the function was broken and should be deprecated.
The idea that session_regenerate_id() should do anything other than regenerate the id for the
current session is totally wrong as that is handled separately by other functions.
I now notice that you have updated this function to delete the current session data as an option,
which is acceptable. The idea that it should do so automatically is what I was objecting to.
------------------------------------------------------------------------
[2018-06-01 10:50:31] yohgaki@php.net
Tony, you are ignoring the fact that there is $delete_old_session option. It exists for security
reasons.
Please stop insisting irrelevant/unrelated opinion for this bug.
------------------------------------------------------------------------
[2018-05-31 12:03:22] tony at marston-home dot demon dot co dot uk
Deleting session data in the file system is what session_destroy() is for.
Use unset($_SESSION) to clear the session data in memory.
Use session_destroy() to clear the session data from the file system.
Both of these operations are completely independent of session_regenerate_id() which should not
clear anything. All it is supposed to do is update the current session_id. Other operations should
be handled with other function calls.
Those users who think that a single function call combines several operations are simply wrong. By
changing this function to automatically include another function will cause problems for those users
who do not want that other function called at all.
------------------------------------------------------------------------
[2018-05-31 11:51:44] yohgaki@php.net
Precisely, there are 2 issues;
1. session_regenerate_id() will not delete/invalidate old data.
2. session_regenerate_id(true) deletes data immediately.
1. allows attackers to abuse hijacked sessions safely(undetected) and freely(keep it as long as they
want).
2. causes race condition that results in lost sessions.
------------------------------------------------------------------------
[2018-05-31 11:34:32] yohgaki@php.net
It's not about $_SESSION, but old session data stored in session storage.
Anyway, my bug description was incorrect.
bool session_regenerate_id ([ bool $delete_old_session = FALSE ] )
$delete_old_session option deletes session data from the session storage immediately, but it should
eventually delete old session data. i.e. It should make old session data inaccessible after a while
by keeping track expiration time stamp to avoid and detect hijacked sessions.
Immediate session data deletion will cause race condition that is very hard to debug also. e.g. Few
lost sessions with tens of millions requests per day.
Current behavior and design is wrong. I created 2 RFCs to fix this design bug. I shall write 3rd RFC
in the future.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=65746
--
Edit this bug report at https://bugs.php.net/bug.php?id=65746&edit=1