diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py index 467d5f1265..ffea036c5e 100644 --- a/django/contrib/sessions/backends/cache.py +++ b/django/contrib/sessions/backends/cache.py @@ -25,7 +25,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): @@ -45,6 +45,8 @@ class SessionStore(SessionBase): raise RuntimeError("Unable to create a new session key.") def save(self, must_create=False): + if self.session_key is None: + return self.create() if must_create: func = self._cache.add else: @@ -56,7 +58,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 ff6076df77..e15f88358c 100644 --- a/django/contrib/sessions/backends/cached_db.py +++ b/django/contrib/sessions/backends/cached_db.py @@ -30,11 +30,12 @@ class SessionStore(DBStore): data = None if data is None: data = super(SessionStore, self).load() - cache.set(self.cache_key, data, settings.SESSION_COOKIE_AGE) + if self.session_key: + cache.set(self.cache_key, data, settings.SESSION_COOKIE_AGE) 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 3dd0d9516c..ad92baee92 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -20,7 +20,7 @@ class SessionStore(SessionBase): ) return self.decode(force_unicode(s.session_data)) except (Session.DoesNotExist, SuspiciousOperation): - self.create() + self._session_key = None return {} def exists(self, session_key): @@ -37,7 +37,6 @@ class SessionStore(SessionBase): # Key wasn't unique. Try again. continue self.modified = True - self._session_cache = {} return def save(self, must_create=False): @@ -47,6 +46,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 8ffddc4903..05dc194f77 100644 --- a/django/contrib/sessions/backends/file.py +++ b/django/contrib/sessions/backends/file.py @@ -56,11 +56,11 @@ class SessionStore(SessionBase): try: session_data = self.decode(file_data) except (EOFError, SuspiciousOperation): - self.create() + self._session_key = None finally: session_file.close() except IOError: - self.create() + self._session_key = None return session_data def create(self): @@ -71,10 +71,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 7686bd254e..98271f4b0e 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -162,6 +162,11 @@ class SessionTestsMixin(object): self.assertNotEqual(self.session.session_key, prev_key) self.assertEqual(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. @@ -256,6 +261,20 @@ class SessionTestsMixin(object): encoded = self.session.encode(data) self.assertEqual(self.session.decode(encoded), data) + 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('deadbeef') + session.load() + + self.assertFalse(session.exists(session.session_key)) + # provided unknown key was cycled, not reused + self.assertNotEqual(session.session_key, 'deadbeef') class DatabaseSessionTests(SessionTestsMixin, TestCase): diff --git a/docs/releases/1.4.21.txt b/docs/releases/1.4.21.txt index 6ff4c6d115..da69b26564 100644 --- a/docs/releases/1.4.21.txt +++ b/docs/releases/1.4.21.txt @@ -5,3 +5,24 @@ Django 1.4.21 release notes *July 8, 2015* Django 1.4.21 fixes several security issues in 1.4.20. + +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.