? Success

User tests: Successful: Unsuccessful:

avatar shandak
shandak
4 Jan 2016

When session time was expired and we open site page, it getting data from expired session data which store in DB, so page has an old form token, and, after, expired data deletes from db here https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/application/cms.php#L758-L768, so next any pages have new token and, when we call ajax from page with old token, we get invalid token message, because user data session was recreated with new token.

avatar shandak shandak - open - 4 Jan 2016
avatar shandak shandak - change - 4 Jan 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Jan 2016
Labels Added: ?
avatar mbabker
mbabker - comment - 4 Jan 2016

The existing behavior is correct. This hack will introduce a security issue of allowing an expired session's data to be considered valid.

avatar roland-d
roland-d - comment - 4 Jan 2016

I agree with @mbabker that current behaviour is correct. If you have an AJAX call with an old token, you should also refresh that page.

avatar shandak
shandak - comment - 4 Jan 2016

At this moment expired session data use for start session anyway, i just fix it for ajax requests, which must use the same token.

avatar shandak
shandak - comment - 4 Jan 2016

In thats code https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/application/cms.php#L750-L776 session starting with an expired data from DB and after was deleted and created again with the same session data, but token was changed, because its random generated.
User session will deletes when expired time for 'remember me' flag. I not see unsecure in my code, because its related with 'remember me' plugin and should be continue user session data here.

avatar mbabker
mbabker - comment - 4 Jan 2016

The code you highlighted has nothing to do with the actual session data other than the $session->start() call. That's all related to a bunch of basically useless extraneous metadata. The session API correctly determines that a session has expired and deals with the mitigation/deletion of data as appropriate.

If the remember me plugin requires this hack to work, then the changes need to be made in the plugin as that is the place modifying the session and user authentication behaviors. The place that you have made the change introduces a security concern into the database session handler which enables ALL sessions with expired data to be resumed.

avatar brianteeman brianteeman - change - 4 Jan 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-01-04 17:28:36
Closed_By brianteeman
avatar brianteeman brianteeman - close - 4 Jan 2016
avatar brianteeman brianteeman - close - 4 Jan 2016
avatar brianteeman
brianteeman - comment - 4 Jan 2016

Closing for the reasons resistance above. It is never going to be accepted as it opens an security hole


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8834.

avatar shandak
shandak - comment - 5 Jan 2016

@mbabker actualy session will delete for any user in the same hightlight code, when another any user come in site. So your notice which enables ALL sessions with expired data to be resumed not true, its not deletes just if native user come in and 'remember me' flag still not expired in user cookie.
And as i say before $session->start() use it anyway with the completley the same useless extraneous metadata.

I see situation: if we use old session data, then it still correct, so session time should have current time. Actualy it do the same now, just recreated data again, so token was changed too.

avatar mbabker
mbabker - comment - 5 Jan 2016

No!

First, the highlighted code is a requirement because of the extra metadata stored by the application regardless of the session store you're using and basically performs the same function as the PHP session API's gc() method. It ensures that even if you're using a non-database session handler, the session table is cleaned up of what should be dead session records. It's crap code, I've been on a crusade against it for months, but frankly without dropping several end user features that have little to no value in 2016 it isn't going anywhere. Set your session handler to anything but the database and see all that extra data that's still written to the database because of JApplicationCms::checkSession().

Your proposal, as I stated previously, introduces a security hole into the database session handler. It introduces the possibility to resume in full an expired session. See https://www.owasp.org/index.php/Session_Management_Cheat_Sheet#Session_Expiration for further guidance on how expired sessions should be handled.

Even with the remember me flag enabled, once the session expires, it would not be resumed. Rather, a new session is created automatically based on valid data to create the remember me token and user cookie. It does not in any way re-use the user's previous session data.

The behavior you are seeing with the invalid CSRF token is therefore fully valid and expected.

avatar shandak
shandak - comment - 5 Jan 2016
  1. session->start getting completeley the same data in first time, so you use expired data anyway, i not found any place which it can be some clears. And it used old token here too.
  2. If you use 'remember me', then it store the same session_id until it not expired, so its client problem how many set time , and then secure, for remember session user.
  3. If user visiting site long time, then his session still continue, so it unsecure too with my or without my changes
  4. Even after deletes session data in DB and store thats again in DB - it store all the same(the same session_id, client_id and session data), differense just in new form token.

So all unsecure what i have in my code - the same form token usage or you see something more?

Add a Comment

Login with GitHub to post a comment