Issue Type: Bug Created: 2008-10-30T15:03:48.000+0000 Last Updated: 2012-05-05T02:43:07.000+0000 Status: Closed Fix version(s): Reporter: Kevin McArthur (kevin) Assignee: Adam Lundrigan (adamlundrigan) Tags: - Zend_Session
Related issues: Attachments:
When using persistent cookies and multiple session namespaces, setExpirationSeconds does not expire data unless the session is resumed. Abandoned sessions will leave data in the sess_ files well beyond their expire time.
Requested fix is to modify the garbage collection handler to search through sessions and delete expired data.
Posted by Steve Hollis (stevehollis) on 2009-11-20T10:05:42.000+0000
The behaviour described is simply how sessions work in PHP (see http://php.net/manual/en/ref.session.php).
Sessions are expired at runtime based on the gc_* family of configuration options. If PHP is not active, garbage collection will not run and the sessions will remain indefinitely.
If I have misunderstood the issue, please provide code to reproduce.
Posted by Kevin McArthur (kevin) on 2009-11-20T10:18:14.000+0000
This does not have to do with gc_ probability. It has to do with expiration seconds (a ZFW feature).
Posted by Steve Hollis (stevehollis) on 2009-11-20T10:38:37.000+0000
Please provide code to reproduce.
Posted by Kevin McArthur (kevin) on 2009-11-20T11:43:51.000+0000
Don't have any offhand -- but this has to do with the mechanism by which data is expired in Zend_Session's setExpirationSeconds call. Note that this bug was filed over a year ago.
Basically, and if i remember what was going wrong last year..... setExpirationSeconds is used so that zend_session will delete the data after x seconds... and it will, if that SAME session is reopened. If not, that data will exist until general garbage collection on the server. (Delete on session resume) as such, session data that doesn't get resumed doesn't get deleted on the right schedule.
This problem is amplified by long-lived gc expiry cycles, like when using pesistent cookies to pickup sessions potentially weeks later.
The fix is to have the resumption of any session to fire into the zfw-specific gc (based on the probability) and to search for data within the sessions that should be expired/removed with the setExpirationSeconds mechanism.
This came up when I was trying to make a dual-zone session... with a data area for shopping cart entries (long lived) and a data area for checkout data (short lived)... with this bug, there's no way to get rid of the short lived data while retaining the long lived data, if that same session is not renewed. As a result, secure data was ending up in the session storage for far longer than was acceptable to the setExpirationSeconds policy.
Hope that clears it up.
Posted by Steve Hollis (stevehollis) on 2009-11-20T12:30:38.000+0000
Thanks for the additional explanation.
The problem is that garbage collection is, by default and unless otherwise a different save handler is specified, carried out by PHP's default session handler and not by ZF. So in actual fact there is no ZF specific GC... if a save handler is not specified, it's left up to PHP to perform read/write/gc etc. Only when a session is accessed and analysed by ZF can it see and act upon an 'expired' (as defined by setExpirationSeconds) session.
The reverse of the issue raised is when the gc_maxlifetime is shorter than setExpirationSeconds. In that situation, sessions will likely be expired before the value in setExpirationSeconds is reached, thus rendering it useless.
The primary role of Zend_Session is to provide a convenient wrapper and namespace functionality to PHP sessions, not replace or alter the core functionality of ext/session. Perhaps this could be made clearer in the documentation - session problems are notoriously difficult to pin down and the lack of clarity here possibly muddies the water unnecessarily.
Posted by Kevin McArthur (kevin) on 2009-11-20T12:47:00.000+0000
The ZFW gc is actually part of the constructor, and has nothing to do with php's GC... this handles the expiration seconds and the hop stuff. It is there where this patch needs to be added, and it needs to look beyond its own sesion namespace.
Posted by Steve Hollis (stevehollis) on 2009-11-20T15:23:43.000+0000
Only available constructor in Zend_Session_* is Zend_Session_Namespace (as you'd expect):
<pre class="highlight"> public function __construct($namespace = 'Default', $singleInstance = false)
I understand that, on the surface, if feels like setExpirationSeconds should do what you're suggesting, but for the reasons explained above, it simply can't and a patch won't help.
The only thing that could incorporate this kind of logic is a default save handler that overrides PHP's default. Since ZF's policy is to not reproduce functionality that already exists in PHP, this seems unlikely to happen.
Posted by Kevin McArthur (kevin) on 2009-11-20T16:07:03.000+0000
You're incorrect on this issue.
If you trace how setExpriationSeconds works you'll notice that from the namespace constructor theres a call into Zend_Session::start... which then eventually calls the _process private methods in Zend_Session...
specifically private static function processStartupMetadataGlobal() is responsible for processing the time limit. To _fix this bug... no save handler needs to be added, just this metadata function expanded to look for other sessions as well as the one currently being operated on. In essence a garbage collector -- which should probably be entered into on a probability.
The expiration seconds mechanism has NOTHING to do with the session lifetime from a php perspective.
Posted by Steve Hollis (stevehollis) on 2009-11-21T01:01:53.000+0000
Indeed, the expiration seconds mechanism has nothing to do with the session lifetime, but since they do effectively the same thing it's important to understand how they work and why.
Put simply, there is no way to operate on a session other than the current one from within a PHP script. That is the job of the save handler. If you feel that statement is incorrect, please provide a code snippet which illustrates how you think it could be achieved.
As the loop in Zend_Session::_processStartupMetadataGlobal() shows, the setExpirationSeconds value is set in a session namespace's metadata and specific to a namespace ("setExpirationSeconds() - expire the namespace, or specific variables after a specified"). Even if it were possible/desirable/correct to examine other sessions here, the performance hit would be potentially huge, since every session would need to be read and each namespace examined for an expired value. On busy production websites this could be tens or hundreds of thousands of sessions.
If you really wanted to do this, you could write a custom save handler whose gc method read all available sessions and checked each namespace individually for expiry. Alternatively, as a potentially more sensible approach, perhaps you could modify the DbTable savehandler to contain a column with the soonest namespace expiry value.
Either way, despite being a bit confusing, this is not a bug.
Posted by Pádraic Brady (padraic) on 2011-08-14T18:35:37.000+0000
Global session expiration and setExpirationSeconds() are independent processes. Global GC is the responsibility of PHP and/or save handlers.
The distinction should be better documented probably since confusing the two can certainly lead to valid security problems which the Zend Framework should do its utmost to avoid. I'll pick it up for ZF2 docs and do something with ZF1 in the near future.
Posted by Kevin McArthur (kevin) on 2011-08-14T19:15:29.000+0000
Can we not close bugs until there is a fix in code. I still consider this a valid bug, it results in a security scenario where data can live on the server beyond the expiration seconds. Its not a communication issue. Its a problem with the code. If ZFW is going to implement an expiration method then it needs a probability based garbage collector to ensure that sessions are actually expired on disk near the 'expiration seconds' demarcation. This has nothing to do with PHP session lifetime which has its own, independent settings, controlls and garbage collection.
This remains an open bug with security implications.
Posted by Kevin McArthur (kevin) on 2011-08-14T19:23:06.000+0000
"Put simply, there is no way to operate on a session other than the current one from within a PHP script. That is the job of the save handler. If you feel that statement is incorrect, please provide a code snippet which illustrates how you think it could be achieved."
This statement is incorrect, the ZFW session is distinct from the PHP session. The sessions can be looked at on disk or via session_id()/session_start() and session_write_close().
The Zend-Specific GC should be entered via a probability. It should register a shutdown handler, (such that the page request is not delayed) and it should loop through the list of active sessions using setExpirationSeconds which, with minor modifications, could be tracked in a separate zend-specific hash table.
One day I'll get around to fixing this bug, but until then, please leave it open and or correct it properly in code.
Posted by Pádraic Brady (padraic) on 2011-08-14T20:40:25.000+0000
I'll make you a deal. I won't close it, again, if you report it to the zf-security email address for resolution within a timeframe you deem acceptable. If it is a valid security concern, it should not be left on the issue tracker for years at a time to be taken advantage of.
Posted by Kevin McArthur (kevin) on 2011-08-14T20:43:30.000+0000
Fair enough, i'll post a note there to review ZF-4753.
Posted by Adam Lundrigan (adamlundrigan) on 2011-10-17T19:25:42.000+0000
Has there been any progress/decision on this issue from zf-security?
Posted by Adam Lundrigan (adamlundrigan) on 2012-03-13T19:20:34.000+0000
I'm going to take that as a no. IMHO, this may not necessarily need a fix this late in ZFv1's lifecycle, but there should at least be a "here be dragons" cautionary tale added to the manual so at least devs can make an informed decision and take any mitigating actions they feel are necessary.
Have you found an issue?
See the Overview section for more details.