Merge pull request #3325 from mmerickel/fix-unpickle-crash

catch other pickle errors when loading content

3 files changed, 68 insertions, 1 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 91dadfa79..9bfa80f05 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -67,6 +67,12 @@ Bug Fixes
   code. The old ``MIMEAccept`` has been deprecated. The new methods follow the
   RFC's more closely. See https://github.com/Pylons/pyramid/pull/3251
 
+- Catch extra errors like ``AttributeError`` when unpickling "trusted"
+  session cookies with bad pickle data in them. This would occur when sharing
+  a secret between projects that shouldn't actually share session cookies,
+  like when reusing secrets between projects in development.
+  See https://github.com/Pylons/pyramid/pull/3325
+
 Deprecations
 ------------
 
diff --git a/pyramid/session.py b/pyramid/session.py
index d05ac66eb..97039a404 100644
--- a/pyramid/session.py
+++ b/pyramid/session.py
@@ -121,7 +121,11 @@ class PickleSerializer(object):
 
     def loads(self, bstruct):
         """Accept bytes and return a Python object."""
-        return pickle.loads(bstruct)
+        try:
+            return pickle.loads(bstruct)
+        # at least ValueError, AttributeError, ImportError but more to be safe
+        except Exception:
+            raise ValueError
 
     def dumps(self, appstruct):
         """Accept a Python object and return bytes."""
diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py
index e3d819944..3585ed635 100644
--- a/pyramid/tests/test_session.py
+++ b/pyramid/tests/test_session.py
@@ -2,6 +2,7 @@ import base64
 import json
 import unittest
 from pyramid import testing
+from pyramid.compat import pickle
 
 class SharedCookieSessionTests(object):
 
@@ -462,6 +463,24 @@ class TestSignedCookieSession(SharedCookieSessionTests, unittest.TestCase):
         self.assertEqual(result, None)
         self.assertTrue('Set-Cookie' in dict(response.headerlist))
 
+    def test_bad_pickle(self):
+        import base64
+        import hashlib
+        import hmac
+
+        digestmod = lambda: hashlib.new('sha512')
+        # generated from dumping an object that cannot be found anymore, eg:
+        # class Foo: pass
+        # print(pickle.dumps(Foo()))
+        cstruct = b'(i__main__\nFoo\np0\n(dp1\nb.'
+        sig = hmac.new(b'pyramid.session.secret', cstruct, digestmod).digest()
+        cookieval = base64.urlsafe_b64encode(sig + cstruct).rstrip(b'=')
+
+        request = testing.DummyRequest()
+        request.cookies['session'] = cookieval
+        session = self._makeOne(request, secret='secret')
+        self.assertEqual(session, {})
+
 class TestUnencryptedCookieSession(SharedCookieSessionTests, unittest.TestCase):
     def setUp(self):
         super(TestUnencryptedCookieSession, self).setUp()
@@ -661,6 +680,44 @@ class Test_signed_deserialize(unittest.TestCase):
         self.assertEqual(result, '123')
 
 
+class TestPickleSerializer(unittest.TestCase):
+    def _makeOne(self):
+        from pyramid.session import PickleSerializer
+        return PickleSerializer()
+
+    def test_loads(self):
+        # generated from dumping Dummy() using protocol=2
+        cstruct = b'\x80\x02cpyramid.tests.test_session\nDummy\nq\x00)\x81q\x01.'
+        serializer = self._makeOne()
+        result = serializer.loads(cstruct)
+        self.assertIsInstance(result, Dummy)
+
+    def test_loads_raises_ValueError_on_invalid_data(self):
+        cstruct = b'not pickled'
+        serializer = self._makeOne()
+        self.assertRaises(ValueError, serializer.loads, cstruct)
+
+    def test_loads_raises_ValueError_on_bad_import(self):
+        # generated from dumping an object that cannot be found anymore, eg:
+        # class Foo: pass
+        # print(pickle.dumps(Foo()))
+        cstruct = b'(i__main__\nFoo\np0\n(dp1\nb.'
+        serializer = self._makeOne()
+        self.assertRaises(ValueError, serializer.loads, cstruct)
+
+    def test_dumps(self):
+        obj = Dummy()
+        serializer = self._makeOne()
+        result = serializer.dumps(obj)
+        expected_result = pickle.dumps(obj, protocol=pickle.HIGHEST_PROTOCOL)
+        self.assertEqual(result, expected_result)
+        self.assertIsInstance(result, bytes)
+
+
+class Dummy(object):
+    pass
+
+
 class DummySerializer(object):
     def dumps(self, value):
         return base64.b64encode(json.dumps(value).encode('utf-8'))