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

Michael Merickel

2018-08-10 5488da0d165468199f0d07a2ca579b86617fad72

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

catch other pickle errors when loading content

 CHANGES.rst

@@ -67,6 +67,12 @@
  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
------------


 pyramid/session.py

@@ -121,7 +121,11 @@

    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."""

 pyramid/tests/test_session.py

@@ -2,6 +2,7 @@
import json
import unittest
from pyramid import testing
from pyramid.compat import pickle

class SharedCookieSessionTests(object):

@@ -462,6 +463,24 @@
        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 @@
        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'))