summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCarl Meyer <carl@oddbird.net>2015-06-10 15:45:20 -0600
committerTim Graham <timograham@gmail.com>2015-07-08 07:35:39 -0400
commit1828f4341ec53a8684112d24031b767eba557663 (patch)
treead2315e49ea8c762203667c2b3a901bc2f268e12
parent9eda28ac48bea8aab1cf622b0e493a66c6b85840 (diff)
downloaddjango-1828f4341ec53a8684112d24031b767eba557663.tar.gz
[1.7.x] Fixed #19324 -- Avoided creating a session record when loading the session.
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. This is a security fix; disclosure to follow shortly.
-rw-r--r--django/contrib/sessions/backends/cache.py6
-rw-r--r--django/contrib/sessions/backends/cached_db.py4
-rw-r--r--django/contrib/sessions/backends/db.py5
-rw-r--r--django/contrib/sessions/backends/file.py5
-rw-r--r--django/contrib/sessions/tests.py20
-rw-r--r--docs/releases/1.4.21.txt21
-rw-r--r--docs/releases/1.7.9.txt21
7 files changed, 74 insertions, 8 deletions
diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py
index b1058b3294..faeb1060d6 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 f5c14b0e1e..5cc6f79b5a 100644
--- a/django/contrib/sessions/backends/cached_db.py
+++ b/django/contrib/sessions/backends/cached_db.py
@@ -51,12 +51,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 self._cache:
+ if session_key and (KEY_PREFIX + session_key) in self._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 a087061c1f..3e6cdf9e9f 100644
--- a/django/contrib/sessions/backends/db.py
+++ b/django/contrib/sessions/backends/db.py
@@ -26,7 +26,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):
@@ -43,7 +43,6 @@ class SessionStore(SessionBase):
# Key wasn't unique. Try again.
continue
self.modified = True
- self._session_cache = {}
return
def save(self, must_create=False):
@@ -53,6 +52,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 6569dafbe3..f886bcd14e 100644
--- a/django/contrib/sessions/backends/file.py
+++ b/django/contrib/sessions/backends/file.py
@@ -96,7 +96,7 @@ class SessionStore(SessionBase):
self.delete()
self.create()
except (IOError, SuspiciousOperation):
- self.create()
+ self._session_key = None
return session_data
def create(self):
@@ -107,10 +107,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 8d63aaa989..6e042c7cc6 100644
--- a/django/contrib/sessions/tests.py
+++ b/django/contrib/sessions/tests.py
@@ -171,6 +171,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):
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.
diff --git a/docs/releases/1.7.9.txt b/docs/releases/1.7.9.txt
index a2101ad6f9..f402a4cdd5 100644
--- a/docs/releases/1.7.9.txt
+++ b/docs/releases/1.7.9.txt
@@ -6,6 +6,27 @@ Django 1.7.9 release notes
Django 1.7.9 fixes several security issues and bugs in 1.7.8.
+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.
+
Bugfixes
========