1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
|
commit 9dfb04b1709e015eff82c5b647f1f0019dc3958b
Author: Tim Graham <timograham@gmail.com>
Date: Wed Aug 5 17:44:48 2015 -0400
[1.4.x] Fixed DoS possiblity in contrib.auth.views.logout()
Refs #20936 -- When logging out/ending a session, don't create a new, empty session.
Previously, when logging out, the existing session was overwritten by a
new sessionid instead of deleting the session altogether.
This behavior added overhead by creating a new session record in
whichever backend was in use: db, cache, etc.
This extra session is unnecessary at the time since no session data is
meant to be preserved when explicitly logging out.
Backport of 393c0e24223c701edeb8ce7dc9d0f852f0c081ad,
088579638b160f3716dc81d194be70c72743593f, and
2dee853ed4def42b7ef1b3b472b395055543cc00 from master
Thanks Florian Apolloner and Carl Meyer for review.
This is a security fix.
Index: python-django-1.4.5/django/contrib/sessions/backends/base.py
===================================================================
--- python-django-1.4.5.orig/django/contrib/sessions/backends/base.py
+++ python-django-1.4.5/django/contrib/sessions/backends/base.py
@@ -128,6 +128,13 @@ class SessionBase(object):
self.accessed = True
self.modified = True
+ def is_empty(self):
+ "Returns True when there is no session_key and the session is empty"
+ try:
+ return not bool(self._session_key) and not self._session_cache
+ except AttributeError:
+ return True
+
def _get_new_session_key(self):
"Returns session key that isn't being used."
# Todo: move to 0-9a-z charset in 1.5
@@ -230,7 +237,7 @@ class SessionBase(object):
"""
self.clear()
self.delete()
- self.create()
+ self._session_key = None
def cycle_key(self):
"""
Index: python-django-1.4.5/django/contrib/sessions/backends/cached_db.py
===================================================================
--- python-django-1.4.5.orig/django/contrib/sessions/backends/cached_db.py
+++ python-django-1.4.5/django/contrib/sessions/backends/cached_db.py
@@ -58,4 +58,4 @@ class SessionStore(DBStore):
"""
self.clear()
self.delete(self.session_key)
- self.create()
+ self._session_key = None
Index: python-django-1.4.5/django/contrib/sessions/middleware.py
===================================================================
--- python-django-1.4.5.orig/django/contrib/sessions/middleware.py
+++ python-django-1.4.5/django/contrib/sessions/middleware.py
@@ -14,30 +14,38 @@ class SessionMiddleware(object):
def process_response(self, request, response):
"""
If request.session was modified, or if the configuration is to save the
- session every time, save the changes and set a session cookie.
+ session every time, save the changes and set a session cookie or delete
+ the session cookie if the session has been emptied.
"""
try:
accessed = request.session.accessed
modified = request.session.modified
+ empty = request.session.is_empty()
except AttributeError:
pass
else:
- if accessed:
- patch_vary_headers(response, ('Cookie',))
- if modified or settings.SESSION_SAVE_EVERY_REQUEST:
- if request.session.get_expire_at_browser_close():
- max_age = None
- expires = None
- else:
- max_age = request.session.get_expiry_age()
- expires_time = time.time() + max_age
- expires = cookie_date(expires_time)
- # Save the session data and refresh the client cookie.
- request.session.save()
- response.set_cookie(settings.SESSION_COOKIE_NAME,
- request.session.session_key, max_age=max_age,
- expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
- path=settings.SESSION_COOKIE_PATH,
- secure=settings.SESSION_COOKIE_SECURE or None,
- httponly=settings.SESSION_COOKIE_HTTPONLY or None)
+ # First check if we need to delete this cookie.
+ # The session should be deleted only if the session is entirely empty
+ if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
+ response.delete_cookie(settings.SESSION_COOKIE_NAME,
+ domain=settings.SESSION_COOKIE_DOMAIN)
+ else:
+ if accessed:
+ patch_vary_headers(response, ('Cookie',))
+ if (modified or settings.SESSION_SAVE_EVERY_REQUEST) and not empty:
+ if request.session.get_expire_at_browser_close():
+ max_age = None
+ expires = None
+ else:
+ max_age = request.session.get_expiry_age()
+ expires_time = time.time() + max_age
+ expires = cookie_date(expires_time)
+ # Save the session data and refresh the client cookie.
+ request.session.save()
+ response.set_cookie(settings.SESSION_COOKIE_NAME,
+ request.session.session_key, max_age=max_age,
+ expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
+ path=settings.SESSION_COOKIE_PATH,
+ secure=settings.SESSION_COOKIE_SECURE or None,
+ httponly=settings.SESSION_COOKIE_HTTPONLY or None)
return response
Index: python-django-1.4.5/django/contrib/sessions/tests.py
===================================================================
--- python-django-1.4.5.orig/django/contrib/sessions/tests.py
+++ python-django-1.4.5/django/contrib/sessions/tests.py
@@ -150,6 +150,7 @@ class SessionTestsMixin(object):
self.session.flush()
self.assertFalse(self.session.exists(prev_key))
self.assertNotEqual(self.session.session_key, prev_key)
+ self.assertIsNone(self.session.session_key)
self.assertTrue(self.session.modified)
self.assertTrue(self.session.accessed)
@@ -432,6 +433,75 @@ class SessionMiddlewareTests(unittest.Te
self.assertNotIn('httponly',
str(response.cookies[settings.SESSION_COOKIE_NAME]))
+ def test_session_delete_on_end(self):
+ request = RequestFactory().get('/')
+ response = HttpResponse('Session test')
+ middleware = SessionMiddleware()
+
+ # Before deleting, there has to be an existing cookie
+ request.COOKIES[settings.SESSION_COOKIE_NAME] = 'abc'
+
+ # Simulate a request that ends the session
+ middleware.process_request(request)
+ request.session.flush()
+
+ # Handle the response through the middleware
+ response = middleware.process_response(request, response)
+
+ # Check that the cookie was deleted, not recreated.
+ # A deleted cookie header looks like:
+ # Set-Cookie: sessionid=; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/
+ self.assertEqual(
+ 'Set-Cookie: %s=; expires=Thu, 01-Jan-1970 00:00:00 GMT; '
+ 'Max-Age=0; Path=/' % settings.SESSION_COOKIE_NAME,
+ str(response.cookies[settings.SESSION_COOKIE_NAME])
+ )
+
+ @override_settings(SESSION_COOKIE_DOMAIN='.example.local')
+ def test_session_delete_on_end_with_custom_domain(self):
+ request = RequestFactory().get('/')
+ response = HttpResponse('Session test')
+ middleware = SessionMiddleware()
+
+ # Before deleting, there has to be an existing cookie
+ request.COOKIES[settings.SESSION_COOKIE_NAME] = 'abc'
+
+ # Simulate a request that ends the session
+ middleware.process_request(request)
+ request.session.flush()
+
+ # Handle the response through the middleware
+ response = middleware.process_response(request, response)
+
+ # Check that the cookie was deleted, not recreated.
+ # A deleted cookie header with a custom domain looks like:
+ # Set-Cookie: sessionid=; Domain=.example.local;
+ # expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/
+ self.assertEqual(
+ 'Set-Cookie: %s=; Domain=.example.local; expires=Thu, '
+ '01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/' % (
+ settings.SESSION_COOKIE_NAME,
+ ),
+ str(response.cookies[settings.SESSION_COOKIE_NAME])
+ )
+
+ def test_flush_empty_without_session_cookie_doesnt_set_cookie(self):
+ request = RequestFactory().get('/')
+ response = HttpResponse('Session test')
+ middleware = SessionMiddleware()
+
+ # Simulate a request that ends the session
+ middleware.process_request(request)
+ request.session.flush()
+
+ # Handle the response through the middleware
+ response = middleware.process_response(request, response)
+
+ # A cookie should not be set.
+ self.assertEqual(response.cookies, {})
+ # The session is accessed so "Vary: Cookie" should be set.
+ self.assertEqual(response['Vary'], 'Cookie')
+
class CookieSessionTests(SessionTestsMixin, TestCase):
Index: python-django-1.4.5/docs/topics/http/sessions.txt
===================================================================
--- python-django-1.4.5.orig/docs/topics/http/sessions.txt
+++ python-django-1.4.5/docs/topics/http/sessions.txt
@@ -197,12 +197,17 @@ You can edit it multiple times.
.. method:: flush
- Delete the current session data from the session and regenerate the
- session key value that is sent back to the user in the cookie. This is
- used if you want to ensure that the previous session data can't be
- accessed again from the user's browser (for example, the
+ Deletes the current session data from the session and deletes the session
+ cookie. This is used if you want to ensure that the previous session data
+ can't be accessed again from the user's browser (for example, the
:func:`django.contrib.auth.logout()` function calls it).
+ .. versionchanged:: 1.4.22
+
+ Deletion of the session cookie was added. Previously, the behavior
+ was to regenerate the session key value that was sent back to the
+ user in the cookie, but this was a denial-of-service vulnerability.
+
.. method:: set_test_cookie
Sets a test cookie to determine whether the user's browser supports
|
