From 2e47f3e401c29bc2ba5ab794d483cb0820855fb9 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 10 Jun 2015 15:45:20 -0600 Subject: [PATCH] [1.4.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. --- django/contrib/sessions/backends/cache.py | 6 ++++-- django/contrib/sessions/backends/cached_db.py | 5 +++-- django/contrib/sessions/backends/db.py | 5 +++-- django/contrib/sessions/backends/file.py | 7 ++++--- django/contrib/sessions/tests.py | 19 +++++++++++++++++++ docs/releases/1.4.21.txt | 21 +++++++++++++++++++++ 6 files changed, 54 insertions(+), 9 deletions(-) --- a/django/contrib/sessions/backends/cache.py +++ b/django/contrib/sessions/backends/cache.py @@ -15,7 +15,7 @@ class SessionStore(SessionBase): session_data = self._cache.get(KEY_PREFIX + self.session_key) if session_data is not None: return session_data - self.create() + self._session_key = None return {} def create(self): @@ -35,6 +35,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: @@ -45,7 +47,7 @@ class SessionStore(SessionBase): raise CreateError def exists(self, session_key): - if self._cache.has_key(KEY_PREFIX + session_key): + if session_key and self._cache.has_key(KEY_PREFIX + session_key): return True return False --- a/django/contrib/sessions/backends/cached_db.py +++ b/django/contrib/sessions/backends/cached_db.py @@ -20,16 +20,19 @@ class SessionStore(DBStore): data = cache.get(KEY_PREFIX + self.session_key, None) if data is None: data = super(SessionStore, self).load() - cache.set(KEY_PREFIX + self.session_key, data, - settings.SESSION_COOKIE_AGE) + if self._session_key: + cache.set(KEY_PREFIX + self._session_key, data, + settings.SESSION_COOKIE_AGE) return data def exists(self, session_key): + if session_key and cache.has_key(KEY_PREFIX + session_key): + return True return super(SessionStore, self).exists(session_key) def save(self, must_create=False): super(SessionStore, self).save(must_create) - cache.set(KEY_PREFIX + self.session_key, self._session, + cache.set(KEY_PREFIX + self.session_key, self._session, settings.SESSION_COOKIE_AGE) def delete(self, session_key=None): @@ -43,4 +46,4 @@ class SessionStore(DBStore): """ self.clear() self.delete(self.session_key) - self.create() \ No newline at end of file + self.create() --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -21,7 +21,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): @@ -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.session_key, session_data = self.encode(self._get_session(no_load=must_create)), --- 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: - pass + 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) --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -40,6 +40,8 @@ False (True, True) >>> db_session['a'], db_session['b'] = 'c', 'd' >>> db_session.save() +>>> db_session['a'] +'c' >>> prev_key = db_session.session_key >>> prev_data = db_session.items() >>> db_session.cycle_key() @@ -55,6 +57,16 @@ True >>> db_session.save() >>> DatabaseSession('1').get('cat') +# Loading an unknown session key does not create a session record +# Creating session records on load is a DOS vulnerability. +>>> session = DatabaseSession('deadbeef') +>>> session.load() +{} +>>> session.exists(session.session_key) +False +>>> session.session_key != 'deadbeef' +True + # # Cached DB session tests # @@ -75,6 +87,20 @@ True >>> cdb_session.delete(cdb_session.session_key) >>> cdb_session.exists(cdb_session.session_key) False +>>> cdb_session['a'] = 'b' +>>> cdb_session.save() +>>> cdb_session['a'] +'b' + +# Loading an unknown session key does not create a session record +# Creating session records on load is a DOS vulnerability. +>>> session = CacheDBSession('deadbeef') +>>> session.load() +{} +>>> session.exists(session.session_key) +False +>>> session.session_key != 'deadbeef' +True # # File session tests. @@ -117,6 +143,8 @@ False (True, True) >>> file_session['a'], file_session['b'] = 'c', 'd' >>> file_session.save() +>>> file_session['a'] +'c' >>> prev_key = file_session.session_key >>> prev_data = file_session.items() >>> file_session.cycle_key() @@ -129,6 +157,16 @@ True >>> file_session = FileSession(file_session.session_key) >>> file_session.save() +# Loading an unknown session key does not create a session record +# Creating session records on load is a DOS vulnerability. +>>> session = FileSession('deadbeef') +>>> session.load() +{} +>>> session.exists(session.session_key) +False +>>> session.session_key != 'deadbeef' +True + # Ensure we don't allow directory traversal >>> FileSession("a/b/c").load() Traceback (innermost last): @@ -186,6 +224,8 @@ False (True, True) >>> cache_session['a'], cache_session['b'] = 'c', 'd' >>> cache_session.save() +>>> cache_session['a'] +'c' >>> prev_key = cache_session.session_key >>> prev_data = cache_session.items() >>> cache_session.cycle_key() @@ -204,6 +244,16 @@ True >>> cache_session.save() >>> cache_session.delete(cache_session.session_key) +# Loading an unknown session key does not create a session record +# Creating session records on load is a DOS vulnerability. +>>> session = CacheSession('deadbeef') +>>> session.load() +{} +>>> session.exists(session.session_key) +False +>>> session.session_key != 'deadbeef' +True + >>> s = SessionBase() >>> s._session['some key'] = 'exists' # Pre-populate the session with some data >>> s.accessed = False # Reset to pretend this wasn't accessed previously