From b769cb9f97ceb95950c7801bc79b13d0f970e9c1 Mon Sep 17 00:00:00 2001
From: Carl Meyer <carl@oddbird.net>
Date: Wed, 10 Jun 2015 15:45:20 -0600
Subject: [PATCH 2/3] [1.6.x] Fixed #19324 -- Avoided creating a session record
 when loading the session. (bnc#937522, CVE-2015-5143)

The session record is now only created if/when the session is modified. This
prevents a potential DoS via creation of many empty session records.

Denial-of-service possibility by filling session store

In previous versions of Django, the session backends created a new empty record in the session storage anytime request.session was accessed and there was a session key provided in the request cookies that didn't already have a session record. This could allow an attacker to easily create many new session records simply by sending repeated requests with unknown session keys, potentially filling up the session store or causing other users' session records to be evicted.

The built-in session backends now create a session record only if the session is actually modified; empty session records are not created. Thus this potential DoS is now only possible if the site chooses to expose a session-modifying view to anonymous users.

As each built-in session backend was fixed separately (rather than a fix in the core sessions framework), maintainers of third-party session backends should check whether the same vulnerability is present in their backend and correct it if so.

Thanks Eric Peterson and Lin Hua Cheng for reporting the issue.

This is a security fix

Conflicts:
	django/contrib/sessions/backends/cached_db.py
	docs/releases/1.4.21.txt
	docs/releases/1.7.9.txt
---
 django/contrib/sessions/backends/cache.py     |  6 ++++--
 django/contrib/sessions/backends/cached_db.py |  4 ++--
 django/contrib/sessions/backends/db.py        |  5 +++--
 django/contrib/sessions/backends/file.py      |  5 +++--
 django/contrib/sessions/tests.py              | 20 ++++++++++++++++++++
 5 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py
index 596042f..87df6be 100644
--- a/django/contrib/sessions/backends/cache.py
+++ b/django/contrib/sessions/backends/cache.py
@@ -27,7 +27,7 @@ class SessionStore(SessionBase):
             session_data = None
         if session_data is not None:
             return session_data
-        self.create()
+        self._session_key = None
         return {}
 
     def create(self):
@@ -49,6 +49,8 @@ class SessionStore(SessionBase):
             "It is likely that the cache is unavailable.")
 
     def save(self, must_create=False):
+        if self.session_key is None:
+            return self.create()
         if must_create:
             func = self._cache.add
         else:
@@ -60,7 +62,7 @@ class SessionStore(SessionBase):
             raise CreateError
 
     def exists(self, session_key):
-        return (KEY_PREFIX + session_key) in self._cache
+        return session_key and (KEY_PREFIX + session_key) in self._cache
 
     def delete(self, session_key=None):
         if session_key is None:
diff --git a/django/contrib/sessions/backends/cached_db.py b/django/contrib/sessions/backends/cached_db.py
index 0fd5a92..a3fbc4d 100644
--- a/django/contrib/sessions/backends/cached_db.py
+++ b/django/contrib/sessions/backends/cached_db.py
@@ -49,12 +49,12 @@ class SessionStore(DBStore):
                     logger = logging.getLogger('django.security.%s' %
                             e.__class__.__name__)
                     logger.warning(force_text(e))
-                self.create()
+                self._session_key = None
                 data = {}
         return data
 
     def exists(self, session_key):
-        if (KEY_PREFIX + session_key) in cache:
+        if session_key and (KEY_PREFIX + session_key) in cache:
             return True
         return super(SessionStore, self).exists(session_key)
 
diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py
index 7be99c3..617bb8b 100644
--- a/django/contrib/sessions/backends/db.py
+++ b/django/contrib/sessions/backends/db.py
@@ -25,7 +25,7 @@ class SessionStore(SessionBase):
                 logger = logging.getLogger('django.security.%s' %
                         e.__class__.__name__)
                 logger.warning(force_text(e))
-            self.create()
+            self._session_key = None
             return {}
 
     def exists(self, session_key):
@@ -42,7 +42,6 @@ class SessionStore(SessionBase):
                 # Key wasn't unique. Try again.
                 continue
             self.modified = True
-            self._session_cache = {}
             return
 
     def save(self, must_create=False):
@@ -52,6 +51,8 @@ class SessionStore(SessionBase):
         create a *new* entry (as opposed to possibly updating an existing
         entry).
         """
+        if self.session_key is None:
+            return self.create()
         obj = Session(
             session_key=self._get_or_create_session_key(),
             session_data=self.encode(self._get_session(no_load=must_create)),
diff --git a/django/contrib/sessions/backends/file.py b/django/contrib/sessions/backends/file.py
index f47aa2d..c857864 100644
--- a/django/contrib/sessions/backends/file.py
+++ b/django/contrib/sessions/backends/file.py
@@ -95,7 +95,7 @@ class SessionStore(SessionBase):
                     self.delete()
                     self.create()
         except (IOError, SuspiciousOperation):
-            self.create()
+            self._session_key = None
         return session_data
 
     def create(self):
@@ -106,10 +106,11 @@ class SessionStore(SessionBase):
             except CreateError:
                 continue
             self.modified = True
-            self._session_cache = {}
             return
 
     def save(self, must_create=False):
+        if self.session_key is None:
+            return self.create()
         # Get the session data now, before we start messing
         # with the file it is stored within.
         session_data = self._get_session(no_load=must_create)
diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py
index 4fb9dbe..912c091 100644
--- a/django/contrib/sessions/tests.py
+++ b/django/contrib/sessions/tests.py
@@ -170,6 +170,11 @@ class SessionTestsMixin(object):
         self.assertNotEqual(self.session.session_key, prev_key)
         self.assertEqual(list(self.session.items()), prev_data)
 
+    def test_save_doesnt_clear_data(self):
+        self.session['a'] = 'b'
+        self.session.save()
+        self.assertEqual(self.session['a'], 'b')
+
     def test_invalid_key(self):
         # Submitting an invalid session key (either by guessing, or if the db has
         # removed the key) results in a new key being generated.
@@ -306,6 +311,21 @@ class SessionTestsMixin(object):
                 self.session.delete(old_session_key)
                 self.session.delete(new_session_key)
 
+    def test_session_load_does_not_create_record(self):
+        """
+        Loading an unknown session key does not create a session record.
+
+        Creating session records on load is a DOS vulnerability.
+        """
+        if self.backend is CookieSession:
+            raise unittest.SkipTest("Cookie backend doesn't have an external store to create records in.")
+        session = self.backend('someunknownkey')
+        session.load()
+
+        self.assertFalse(session.exists(session.session_key))
+        # provided unknown key was cycled, not reused
+        self.assertNotEqual(session.session_key, 'someunknownkey')
+
 
 class DatabaseSessionTests(SessionTestsMixin, TestCase):
 
-- 
2.1.4

