FE Session record is never removed, even if no session data left
One of the security features in TYPO3 is a changing FE cookie IDs for each request. This mechanism is paused when session data is saved. The problem is that there is no proper way to remove session data. Instead, entries are saved without valid data. So even if there is no more session data, the cookie stays unchanged, which undermines a security feature.
Flashmessages demonstrate the issue. Once a flashmessages was set in session data, it never gets removed. Even if it was delivered and flushed.
Invalidation of FE session data happens in tslib_feuserauth::storeSessionData()
... $insertFields = array ( 'hash' => $this->id, 'content' => serialize($this->sesData), 'tstamp' => $GLOBALS['EXEC_TIME'], ); $this->removeSessionData(); $GLOBALS['TYPO3_DB']->exec_INSERTquery('fe_session_data', $insertFields); ...
$this->sesData is NULL, but serialize($this->sesData) results in 'N;'.
However, even if it was NULL, the session data is never removed. There is no check for empty data.
Solution is to check for $data===NULL in setKey(), then unset the key and check for empty $this->sesData before writing to DB.
Updated by Steffen Müller over 9 years ago
Demo extension added.
Just install and add plugin to page.
- Hit "Reload page" multiple times to see the cookie changing.
- Hit "Add and show a Flashmessage" to add flash message.
- Hit "Reload page" again multiple times to see the cookie is NOT changing anymore.
Apply patch and replay above steps.
Updated by Helmut Hummel over 9 years ago
- Status changed from Under Review to Needs Feedback
There's no benefit in changing the session id on every request. This is just a side effect of the session fixation prevention in combination with the currently implemented frontend session data logic.
It would be absolutely fine to have the same session id over several requests, especially when it's an unauthorized session, if it can be guaranteed that it is an id generated by TYPO3.
Just in contrast I would prefer to keep one id for unauthorized sessions and only regenerate it when a user logs in.
I think your patch would not harm, but I also do not see any benefits. Can you elaborate your point?