diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 9401043d78d..ff7bbb0c8a9 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1170,6 +1170,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 55affb2c7c4..42ce62fc7cd 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -5988,23 +5988,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); @@ -6014,6 +6021,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