Commit 1828f434 authored by Carl Meyer's avatar Carl Meyer Committed by Tim Graham
Browse files

[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.
parent 9eda28ac

django/contrib/sessions/backends/cache.py

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

django/contrib/sessions/backends/cached_db.py

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

django/contrib/sessions/backends/db.py

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

django/contrib/sessions/backends/file.py

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

django/contrib/sessions/tests.py

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