Session id isn't destroyed on server after logout

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: 0x5042, Assigned: ygjb)

Tracking

(Blocks: 1 bug, {sec-high, wsec-authentication, wsec-session})

unspecified
sec-high, wsec-authentication, wsec-session
Bug Flags:
sec-bounty +

Details

(Whiteboard: [infrasec:cookie][ws:high])

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
(Assignee)

Comment 1

8 years ago
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
Whiteboard: [infrasec:cookie][ws:normal]
Has this been reported to the django devs?
(Assignee)

Comment 3

8 years ago
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.
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.
(Reporter)

Comment 5

8 years ago
Created attachment 509868 [details]
recorded test scenario
(Reporter)

Comment 6

8 years ago
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.
(Assignee)

Updated

8 years ago
Assignee: nobody → yboily
(Assignee)

Comment 7

7 years ago
We have been able to repro this issue.  I don't know why we weren't able to before.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 9

7 years ago
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.
Whiteboard: [infrasec:cookie][ws:normal] → [infrasec:cookie][ws:critical]
Whiteboard: [infrasec:cookie][ws:critical] → [infrasec:cookie][ws:high]
Duplicate of this bug: 700450
what is the status of this bug?
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.
This is fixed now that PHP is gone.  I verified by watching the sessions table in the database.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

6 years ago
Blocks: 835438
Flags: sec-bounty+
Group: client-services-security
Keywords: wsec-authentication, wsec-session
Keywords: sec-high
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.