From 2eb86b01d7b59be06076f6179a454d0fd0afaff6 Mon Sep 17 00:00:00 2001
From: Tim Graham <timograham@gmail.com>
Date: Wed, 5 Aug 2015 16:51:42 -0400
Subject: [PATCH] [1.8.x] 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 c21036b124..a6c61ac504 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 a1aab37273..220ddb2eda 100644
--- a/tests/sessions_tests/tests.py
+++ b/tests/sessions_tests/tests.py
@@ -660,6 +660,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):