From 8cc41ce7a7a8f6bebfdd89d5ab276cd0109f4fc5 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 5 Aug 2015 16:51:42 -0400 Subject: [PATCH] Fixed DoS possiblity in contrib.auth.views.logout() Thanks Florian Apolloner and Carl Meyer for review. This is a security fix. --- django/contrib/sessions/middleware.py | 2 +- docs/releases/1.4.22.txt | 18 ++++++++++++++++++ docs/releases/1.7.10.txt | 18 ++++++++++++++++++ docs/releases/1.8.4.txt | 13 +++++++++++++ tests/sessions_tests/tests.py | 17 +++++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) diff --git a/django/contrib/sessions/middleware.py b/django/contrib/sessions/middleware.py index 8f12a12623..96751dbbc1 100644 --- a/django/contrib/sessions/middleware.py +++ b/django/contrib/sessions/middleware.py @@ -36,7 +36,7 @@ class SessionMiddleware(object): else: if accessed: patch_vary_headers(response, ('Cookie',)) - if modified or settings.SESSION_SAVE_EVERY_REQUEST: + if (modified or settings.SESSION_SAVE_EVERY_REQUEST) and not empty: if request.session.get_expire_at_browser_close(): max_age = None expires = None diff --git a/docs/releases/1.4.22.txt b/docs/releases/1.4.22.txt index d8ce24bc68..9f8177440f 100644 --- a/docs/releases/1.4.22.txt +++ b/docs/releases/1.4.22.txt @@ -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. diff --git a/docs/releases/1.7.10.txt b/docs/releases/1.7.10.txt index 76457bccbd..38af4a42ce 100644 --- a/docs/releases/1.7.10.txt +++ b/docs/releases/1.7.10.txt @@ -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. diff --git a/docs/releases/1.8.4.txt b/docs/releases/1.8.4.txt index 7b566441e4..e894936bcc 100644 --- a/docs/releases/1.8.4.txt +++ b/docs/releases/1.8.4.txt @@ -6,6 +6,19 @@ Django 1.8.4 release notes Django 1.8.4 fixes a security issue and several bugs in 1.8.3. +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. + Bugfixes ======== diff --git a/tests/sessions_tests/tests.py b/tests/sessions_tests/tests.py index 6af969f70c..713ff9e553 100644 --- a/tests/sessions_tests/tests.py +++ b/tests/sessions_tests/tests.py @@ -678,6 +678,23 @@ class SessionMiddlewareTests(TestCase): 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') + # Don't need DB flushing for these tests, so can use unittest.TestCase as base class class CookieSessionTests(SessionTestsMixin, unittest.TestCase):