629837 - Session id isn't destroyed on server after logout

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[0x5042]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Ubuntu/10.10 Chromium/8.0.552.237 Chrome/8.0.552.237 Safari/534.10
Build Identifier: 

When user logs out cookies with session id are destroyed, but session id on the server side isn't. When browser sends the same sessionid cookie, user is still logged in.

Reproducible: Always

Steps to Reproduce:
1. Login to addons.mozilla.org, 'Remember me on this computer' checkbox should be empty.
2. Type javascript:alert(document.cookie) in url, save sessionid value
3. Logout
4. Use different/restart browser [optional]
5. Go to addons.mozilla.org
6. Modify cookies, set new sessionid cookie with saved value
7. Refresh page
Actual Results:  
Session is restored, user is logged in, can modify account setting etc.

Expected Results:  
Application should destroy session id after logout, user shouldn't be logged in after setting session id cookie from previous session.

Comment 1

[Yvan Boily [:ygjb][:yvan]]

I have investigated this issue and the session is not recovered with this attack.  Django uses a caching framework that allows template content to be generated and cached.  When this occurs, the cache entries are written that may potentially contain sensitive data, but these cache entries are associated with the session identifier.  Unfortunately these cache entries may not be invalidated/cleared when the session is destroyed.

Repeating the steps in the initial report results in a view that is comprised of the normal, unauthenticated view, with the exception of the welcome banner and some profile related menu items.  When any of these menu items is selected, the user is forced to authenticate (expected, since they are simply viewing a cached template). 

The root cause of the bug seems to be in the cache.py[1]; this function implements the delete method required by base.py[2], but the base.flush() method does not pass the sessionid when delete() is called.  As a result the cache.delete() call does not actually delete anything.  The flush method is ultimately called by auth.logout[3], which is in turn called by AMO to log out a user.

[1] django/contrib/sessions/backends/cache.py
[2] django/contrib/sessions/backends/base.py
[3] django/contrib/auth/__init__.py

Comment 2

[Wil Clouser [:clouserw]]

Has this been reported to the django devs?

Comment 3

[Yvan Boily [:ygjb][:yvan]]

Not yet, some additional research by someone who understands how more about how django caching works and knows python better than me should probably take a look before it is reported.  If you or another dev can confirm my conclusions in comment 1, I will report it to Django.

Comment 4

[Jeff Balogh (:jbalogh)]

We use the database backend for sessions, so sessions.backends.cache is nowhere in our code path. 

Template caching is handled through cache-machine completely separately, but no templates are cached for authenticated users.

I'll look at what happens when we call logout.

Comment 5

[0x5042]

Attachment: recorded test scenario

Comment 6

[0x5042]

I have recorded steps to reproduce this issue, today it didn't worked as described in primary report - step 7 has changed: I clicked on ADD-ONS link twice instead of page refresh.

Comment 7

[Yvan Boily [:ygjb][:yvan]]

We have been able to repro this issue.  I don't know why we weren't able to before.

Comment 9

[Yvan Boily [:ygjb][:yvan]]

The root cause of this bug appears to be the integration between the cake and django session management.  The changes to remove cake also remove this issue.

Comment 11

[Raymond Forbes[:rforbes]]

what is the status of this bug?

Comment 12

[Wil Clouser [:clouserw]]

if comment 9 is correct it'll be resolved as soon as PHP is gone.  This Thursday all traffic to PHP is being turned off and then $some_amount_of_time later, once we're satisfied all is well, we can remove it from the servers altogether.

Comment 13

[Wil Clouser [:clouserw]]

This is fixed now that PHP is gone.  I verified by watching the sessions table in the database.