php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #35479 garbage_collector calling sequence
Submitted: 2005-11-29 18:15 UTC Modified: 2018-04-17 10:13 UTC
Votes:2
Avg. Score:4.5 ± 0.5
Reproduced:2 of 2 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: kenashkov at gmail dot com Assigned:
Status: Wont fix Package: Session related
PHP Version: 4.4.1 OS: Fedora Core 4
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2005-11-29 18:15 UTC] kenashkov at gmail dot com
Description:
------------
Let suppose that we use the session_set_save_handler to register own session handling functions and we have an expired session (but not cleaned by the garbage collector yet). When we start the session with session_start() we get the following sequence of calling the registered functions:
open
read 
gc
write
close
I think the garbage collector (gc) should be called BEFORE the read function (in order to clean that expired session beofre it is read). In the way it is, is possible for the web site visitor to use an old session (only once of course, because immediately after read is called gc and for the second visit the session will be already deleted).
Maybe the same problem exists when is not used the session_set_save_handler, but with it the sequence can be seen.

Reproduce code:
---------------
<?
function open() { print 'open<br>'.PHP_EOL; return true; }
function close() { print 'close<br>'.PHP_EOL; return true; }
function read() { print 'read<br>'.PHP_EOL; return ''; }
function write() { print 'write<br>'.PHP_EOL; return true; }
function destroy() { print 'destroy<br>'.PHP_EOL; return true; }
function gc() { print 'gc<br>'.PHP_EOL; return true; }
ini_set('session.gc_probability',1);
ini_set('session.gc_divisor',1);
session_set_save_handler('open','close','read','write','destroy','gc');
session_start();
session_write_close();
?>

Expected result:
----------------
open
gc
read
write
close

Actual result:
--------------
open
read
gc
write
close

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2005-11-29 22:55 UTC] sniper@php.net
It's fine as it is. Changing this now would break backwards compatibility.
 [2005-11-30 20:22 UTC] kenashkov at gmail dot com
I think this can be a major security risk if the garbage collection rule is based not on the session begin time, but on a inactivity time (i.e. 1 hour since last read). Thus when long time there is no any execution of the code (from other users/sessions) is possible a visitor to use an abandoned session (i.e. not closed window on some computer) and execute the page. Then the read function is called, it updates the last activity time, and the gc will not delete the session, because it is already updated. A work around is to update the last activity time in the write function (which is AFTER the gc) but this way remains the possibility to execute at least once an abandoned session. 

And is that the order of calling of the internal functions when is used the default handler? If it is I think it is important not to leave this as is.
Maybe is possible to add php.ini directive that allows switching the calling order. Something like "change_session_call_order" On/Off PHP_INI_ALL. This way if there is no compatibility issues, one can turn it On to get the proposed calling order. I know that there is already a lot of session directives, but this is better than nothing.
 [2018-04-17 07:53 UTC] robert dot schneider at colop dot co dot at
This is not fine as it is, I would say. It's one call to late to find out that the session was garbage collected. The one call where the session gets deleted pretends that the session is still valid - what is not correct. Why should this be a proper way?

I would also declare it as a bug. And I don't think anyone relies to this behaviour. So I don't see a problem with backward compability.

But it also could be resolved with an optional parameter or php setting. 

Please re-open this ticket. Same issue can be found also here: https://bugs.php.net/bug.php?id=35602
 [2018-04-17 10:13 UTC] requinix@php.net
I'm not going to reopen 13 year old bugs for a topic as controversial as sessions. Talk to the internals mailing list.

Session GC is *garbage collection* to clean up expired session data. It is not session security.
If you need the added security then include timestamps in your session data and validate lifetime using code as part of the other session security best practices you should already be using.
http://php.net/features.session.security.management#features.session.security.management.session-data-deletion
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 23:01:27 2024 UTC