diff options
author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2020-07-13 11:17:01 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-07-13 14:17:01 -0400 |
commit | 620e276a8c1d53332fbf08d369be87f862b6949d (patch) | |
tree | 6f793f285211d2c12b6539b2bc767f308b60a30d | |
parent | closes bpo-41235: Fix the error handling in SSLContext.load_dh_params() (GH-2... (diff) | |
download | cpython-620e276a8c1d53332fbf08d369be87f862b6949d.tar.gz cpython-620e276a8c1d53332fbf08d369be87f862b6949d.tar.bz2 cpython-620e276a8c1d53332fbf08d369be87f862b6949d.zip |
bpo-41288: Fix a crash in unpickling invalid NEWOBJ_EX. (GH-21458) (GH-21461)
Automerge-Triggered-By: @tiran
(cherry picked from commit 4f309abf55f0e6f8950ac13d6ec83c22b8d47bf8)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
-rw-r--r-- | Lib/test/pickletester.py | 18 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2020-07-13-15-06-35.bpo-41288.8mn5P-.rst | 2 | ||||
-rw-r--r-- | Modules/_pickle.c | 29 |
3 files changed, 41 insertions, 8 deletions
diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 1d88fcb859a..c576d73349a 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -998,6 +998,24 @@ class AbstractUnpickleTests(unittest.TestCase): self.assertIs(type(unpickled), collections.UserDict) self.assertEqual(unpickled, collections.UserDict({1: 2})) + def test_bad_reduce(self): + self.assertEqual(self.loads(b'cbuiltins\nint\n)R.'), 0) + self.check_unpickling_error(TypeError, b'N)R.') + self.check_unpickling_error(TypeError, b'cbuiltins\nint\nNR.') + + def test_bad_newobj(self): + error = (pickle.UnpicklingError, TypeError) + self.assertEqual(self.loads(b'cbuiltins\nint\n)\x81.'), 0) + self.check_unpickling_error(error, b'cbuiltins\nlen\n)\x81.') + self.check_unpickling_error(error, b'cbuiltins\nint\nN\x81.') + + def test_bad_newobj_ex(self): + error = (pickle.UnpicklingError, TypeError) + self.assertEqual(self.loads(b'cbuiltins\nint\n)}\x92.'), 0) + self.check_unpickling_error(error, b'cbuiltins\nlen\n)}\x92.') + self.check_unpickling_error(error, b'cbuiltins\nint\nN}\x92.') + self.check_unpickling_error(error, b'cbuiltins\nint\n)N\x92.') + def test_bad_stack(self): badpickles = [ b'.', # STOP diff --git a/Misc/NEWS.d/next/Library/2020-07-13-15-06-35.bpo-41288.8mn5P-.rst b/Misc/NEWS.d/next/Library/2020-07-13-15-06-35.bpo-41288.8mn5P-.rst new file mode 100644 index 00000000000..3c3adbabf16 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-07-13-15-06-35.bpo-41288.8mn5P-.rst @@ -0,0 +1,2 @@ +Unpickling invalid NEWOBJ_EX opcode with the C implementation raises now +UnpicklingError instead of crashing. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index ef83da02e2e..329631d7e3b 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -5515,23 +5515,30 @@ load_newobj_ex(UnpicklerObject *self) } if (!PyType_Check(cls)) { - Py_DECREF(kwargs); - Py_DECREF(args); PyErr_Format(st->UnpicklingError, "NEWOBJ_EX class argument must be a type, not %.200s", Py_TYPE(cls)->tp_name); - Py_DECREF(cls); - return -1; + goto error; } if (((PyTypeObject *)cls)->tp_new == NULL) { - Py_DECREF(kwargs); - Py_DECREF(args); - Py_DECREF(cls); PyErr_SetString(st->UnpicklingError, "NEWOBJ_EX class argument doesn't have __new__"); - return -1; + goto error; + } + if (!PyTuple_Check(args)) { + PyErr_Format(st->UnpicklingError, + "NEWOBJ_EX args argument must be a tuple, not %.200s", + Py_TYPE(args)->tp_name); + goto error; + } + if (!PyDict_Check(kwargs)) { + PyErr_Format(st->UnpicklingError, + "NEWOBJ_EX kwargs argument must be a dict, not %.200s", + Py_TYPE(kwargs)->tp_name); + goto error; } + obj = ((PyTypeObject *)cls)->tp_new((PyTypeObject *)cls, args, kwargs); Py_DECREF(kwargs); Py_DECREF(args); @@ -5541,6 +5548,12 @@ load_newobj_ex(UnpicklerObject *self) } PDATA_PUSH(self->stack, obj, -1); return 0; + +error: + Py_DECREF(kwargs); + Py_DECREF(args); + Py_DECREF(cls); + return -1; } static int |