From 10b7da0a156f58ced13e4baba15108eb8723d303 Mon Sep 17 00:00:00 2001
From: Matthias Runge <mrunge@redhat.com>
Date: Fri, 17 Jul 2015 10:54:54 +0200
Subject: [PATCH] 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.
This is a cherry-pick to stable/1.6.x
---
django/contrib/sessions/backends/cache.py | 6 ++++--
django/contrib/sessions/backends/cached_db.py | 14 ++++++++------
django/contrib/sessions/backends/db.py | 6 ++++--
django/contrib/sessions/backends/file.py | 6 ++++--
django/contrib/sessions/tests.py | 21 +++++++++++++++++++++
5 files changed, 41 insertions(+), 12 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 be22c1f..8f2cf81 100644
--- a/django/contrib/sessions/backends/cached_db.py
+++ b/django/contrib/sessions/backends/cached_db.py
@@ -4,6 +4,7 @@ Cached, database-backed sessions.
import logging
+from django.conf import settings
from django.contrib.sessions.backends.db import SessionStore as DBStore
from django.core.cache import cache
from django.core.exceptions import SuspiciousOperation
@@ -19,6 +20,7 @@ class SessionStore(DBStore):
"""
def __init__(self, session_key=None):
+ self._cache = cache
super(SessionStore, self).__init__(session_key)
@property
@@ -27,7 +29,7 @@ class SessionStore(DBStore):
def load(self):
try:
- data = cache.get(self.cache_key, None)
+ data = self._cache.get(self.cache_key, None)
except Exception:
# Some backends (e.g. memcache) raise an exception on invalid
# cache keys. If this happens, reset the session. See #17810.
@@ -42,25 +44,25 @@ class SessionStore(DBStore):
expire_date__gt=timezone.now()
)
data = self.decode(s.session_data)
- cache.set(self.cache_key, data,
+ self._cache.set(self.cache_key, data,
self.get_expiry_age(expiry=s.expire_date))
except (Session.DoesNotExist, SuspiciousOperation) as e:
if isinstance(e, SuspiciousOperation):
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 self._cache:
return True
return super(SessionStore, self).exists(session_key)
def save(self, must_create=False):
super(SessionStore, self).save(must_create)
- cache.set(self.cache_key, self._session, self.get_expiry_age())
+ self._cache.set(self.cache_key, self._session, self.get_expiry_age())
def delete(self, session_key=None):
super(SessionStore, self).delete(session_key)
@@ -68,7 +70,7 @@ class SessionStore(DBStore):
if self.session_key is None:
return
session_key = self.session_key
- cache.delete(KEY_PREFIX + session_key)
+ self._cache.delete(KEY_PREFIX + session_key)
def flush(self):
"""
diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py
index 7be99c3..3e6cdf9 100644
--- a/django/contrib/sessions/backends/db.py
+++ b/django/contrib/sessions/backends/db.py
@@ -6,6 +6,7 @@ from django.db import IntegrityError, transaction, router
from django.utils import timezone
from django.utils.encoding import force_text
+
class SessionStore(SessionBase):
"""
Implements database session store.
@@ -25,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):
@@ -42,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):
@@ -52,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 f47aa2d..f886bcd 100644
--- a/django/contrib/sessions/backends/file.py
+++ b/django/contrib/sessions/backends/file.py
@@ -13,6 +13,7 @@ from django.utils.encoding import force_text
from django.contrib.sessions.exceptions import InvalidSessionKey
+
class SessionStore(SessionBase):
"""
Implements a file based session store.
@@ -95,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):
@@ -106,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 ff0a70c..6a04f24 100644
--- a/django/contrib/sessions/tests.py
+++ b/django/contrib/sessions/tests.py
@@ -160,6 +160,11 @@ class SessionTestsMixin(object):
self.assertTrue(self.session.modified)
self.assertTrue(self.session.accessed)
+ def test_save_doesnt_clear_data(self):
+ self.session['a'] = 'b'
+ self.session.save()
+ self.assertEqual(self.session['a'], 'b')
+
def test_cycle(self):
self.session['a'], self.session['b'] = 'c', 'd'
self.session.save()
@@ -305,6 +310,22 @@ 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.4.3