Skip to content
Permalink
Browse files

[1.7.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 393c0e2,
0885796, and
2dee853 from master

Thanks Florian Apolloner and Carl Meyer for review.

This is a security fix.
  • Loading branch information...
timgraham committed Aug 5, 2015
1 parent 95af894 commit 2f5485346ee6f84b4e52068c04e043092daf55f7
@@ -142,6 +142,13 @@ def clear(self):
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."
while True:
@@ -268,7 +275,7 @@ def flush(self):
"""
self.clear()
self.delete()
self.create()
self._session_key = None

def cycle_key(self):
"""
@@ -79,7 +79,7 @@ def flush(self):
"""
self.clear()
self.delete(self.session_key)
self.create()
self._session_key = None


# At bottom to avoid circular import
@@ -18,32 +18,40 @@ def process_request(self, request):
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.
# Skip session save for 500 responses, refs #3881.
if response.status_code != 500:
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.
# Skip session save for 500 responses, refs #3881.
if response.status_code != 500:
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
@@ -159,6 +159,7 @@ def test_flush(self):
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)

@@ -589,6 +590,75 @@ def test_session_save_on_500(self):
# Check that the value wasn't saved above.
self.assertNotIn('hello', request.session.load())

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: {0}=; expires=Thu, 01-Jan-1970 00:00:00 GMT; '
'Max-Age=0; Path=/'.format(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: {}=; Domain=.example.local; expires=Thu, '
'01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/'.format(
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):

@@ -9,3 +9,21 @@ Django 1.4.22 fixes a security issue in 1.4.21.
It also fixes support with pip 7+ by disabling wheel support. Older versions
of 1.4 would silently build a broken wheel when installed with those versions
of pip.

Denial-of-service possibility in ``logout()`` view by filling session store
===========================================================================

Previously, a session could be created when anonymously accessing the
:func:`django.contrib.auth.views.logout` view (provided it wasn't decorated
with :func:`~django.contrib.auth.decorators.login_required` as done in the
admin). This could allow an attacker to easily create many new session records
by sending repeated requests, potentially filling up the session store or
causing other users' session records to be evicted.

The :class:`~django.contrib.sessions.middleware.SessionMiddleware` has been
modified to no longer create empty session records.

Additionally, the ``contrib.sessions.backends.base.SessionBase.flush()`` and
``cache_db.SessionStore.flush()`` methods have been modified to avoid creating
a new empty session. Maintainers of third-party session backends should check
if the same vulnerability is present in their backend and correct it if so.
@@ -5,3 +5,21 @@ Django 1.7.10 release notes
*August 18, 2015*

Django 1.7.10 fixes a security issue in 1.7.9.

Denial-of-service possibility in ``logout()`` view by filling session store
===========================================================================

Previously, a session could be created when anonymously accessing the
:func:`django.contrib.auth.views.logout` view (provided it wasn't decorated
with :func:`~django.contrib.auth.decorators.login_required` as done in the
admin). This could allow an attacker to easily create many new session records
by sending repeated requests, potentially filling up the session store or
causing other users' session records to be evicted.

The :class:`~django.contrib.sessions.middleware.SessionMiddleware` has been
modified to no longer create empty session records.

Additionally, the ``contrib.sessions.backends.base.SessionBase.flush()`` and
``cache_db.SessionStore.flush()`` methods have been modified to avoid creating
a new empty session. Maintainers of third-party session backends should check
if the same vulnerability is present in their backend and correct it if so.
@@ -226,12 +226,18 @@ 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.7.10

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 could be a denial-of-service
vulnerability.

.. method:: set_test_cookie()

Sets a test cookie to determine whether the user's browser supports

0 comments on commit 2f54853

Please sign in to comment.
You can’t perform that action at this time.