Commit 2e47f3e4 authored by Carl Meyer's avatar Carl Meyer Committed by Tim Graham
Browse files

[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.
parent c570a5ec

django/contrib/sessions/backends/cache.py

+4 −2
Original line number Diff line number Diff line
@@ -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:

django/contrib/sessions/backends/cached_db.py

+3 −2
Original line number Diff line number Diff line
@@ -30,11 +30,12 @@ class SessionStore(DBStore): 
            data = None

        if data is None:

            data = super(SessionStore, self).load()

            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)

django/contrib/sessions/backends/db.py

+3 −2
Original line number Diff line number Diff line
@@ -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)),

django/contrib/sessions/backends/file.py

+4 −3
Original line number Diff line number Diff line
@@ -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)

django/contrib/sessions/tests.py

+19 −0
Original line number Diff line number Diff line
@@ -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):