changeset: 99307:b08c3a733fda parent: 99304:59f33f4249ec parent: 99306:531e2674f003 user: Serhiy Storchaka date: Mon Nov 23 15:20:21 2015 +0200 files: Lib/test/pickletester.py Misc/NEWS Modules/_pickle.c description: Issue #23914: Fixed SystemError raised by unpickler on broken pickle data. diff -r 59f33f4249ec -r b08c3a733fda Lib/test/pickletester.py --- a/Lib/test/pickletester.py Sun Nov 22 23:46:02 2015 -0800 +++ b/Lib/test/pickletester.py Mon Nov 23 15:20:21 2015 +0200 @@ -12,7 +12,7 @@ from http.cookies import SimpleCookie from test.support import ( - TestFailed, TESTFN, run_with_locale, no_tracing, + TestFailed, TESTFN, run_with_locale, no_tracing, captured_stdout, _2G, _4G, bigmemtest, ) @@ -987,6 +987,89 @@ self.assertIs(type(unpickled), collections.UserDict) self.assertEqual(unpickled, collections.UserDict({1: 2})) + def test_bad_stack(self): + badpickles = [ + b'0.', # POP + b'1.', # POP_MARK + b'2.', # DUP + # b'(2.', # PyUnpickler doesn't raise + b'R.', # REDUCE + b')R.', + b'a.', # APPEND + b'Na.', + b'b.', # BUILD + b'Nb.', + b'd.', # DICT + b'e.', # APPENDS + # b'(e.', # PyUnpickler raises AttributeError + b'ibuiltins\nlist\n.', # INST + b'l.', # LIST + b'o.', # OBJ + b'(o.', + b'p1\n.', # PUT + b'q\x00.', # BINPUT + b'r\x00\x00\x00\x00.', # LONG_BINPUT + b's.', # SETITEM + b'Ns.', + b'NNs.', + b't.', # TUPLE + b'u.', # SETITEMS + b'(u.', + b'}(Nu.', + b'\x81.', # NEWOBJ + b')\x81.', + b'\x85.', # TUPLE1 + b'\x86.', # TUPLE2 + b'N\x86.', + b'\x87.', # TUPLE3 + b'N\x87.', + b'NN\x87.', + b'\x90.', # ADDITEMS + # b'(\x90.', # PyUnpickler raises AttributeError + b'\x91.', # FROZENSET + b'\x92.', # NEWOBJ_EX + b')}\x92.', + b'\x93.', # STACK_GLOBAL + b'Vlist\n\x93.', + b'\x94.', # MEMOIZE + ] + for p in badpickles: + with self.subTest(p): + self.assertRaises(self.bad_stack_errors, self.loads, p) + + def test_bad_mark(self): + badpickles = [ + b'cbuiltins\nlist\n)(R.', # REDUCE + b'cbuiltins\nlist\n()R.', + b']N(a.', # APPEND + b'cbuiltins\nValueError\n)R}(b.', # BUILD + b'cbuiltins\nValueError\n)R(}b.', + b'(Nd.', # DICT + b'}NN(s.', # SETITEM + b'}N(Ns.', + b'cbuiltins\nlist\n)(\x81.', # NEWOBJ + b'cbuiltins\nlist\n()\x81.', + b'N(\x85.', # TUPLE1 + b'NN(\x86.', # TUPLE2 + b'N(N\x86.', + b'NNN(\x87.', # TUPLE3 + b'NN(N\x87.', + b'N(NN\x87.', + b'cbuiltins\nlist\n)}(\x92.', # NEWOBJ_EX + b'cbuiltins\nlist\n)(}\x92.', + b'cbuiltins\nlist\n()}\x92.', + b'Vbuiltins\n(Vlist\n\x93.', # STACK_GLOBAL + b'Vbuiltins\nVlist\n(\x93.', + ] + for p in badpickles: + # PyUnpickler prints reduce errors to stdout + with self.subTest(p), captured_stdout(): + try: + self.loads(p) + except (IndexError, AttributeError, TypeError, + pickle.UnpicklingError): + pass + class AbstractPickleTests(unittest.TestCase): # Subclass must define self.dumps, self.loads. diff -r 59f33f4249ec -r b08c3a733fda Lib/test/test_pickle.py --- a/Lib/test/test_pickle.py Sun Nov 22 23:46:02 2015 -0800 +++ b/Lib/test/test_pickle.py Mon Nov 23 15:20:21 2015 +0200 @@ -32,6 +32,7 @@ class PyUnpicklerTests(AbstractUnpickleTests): unpickler = pickle._Unpickler + bad_stack_errors = (IndexError,) def loads(self, buf, **kwds): f = io.BytesIO(buf) @@ -62,6 +63,7 @@ pickler = pickle._Pickler unpickler = pickle._Unpickler + bad_stack_errors = (pickle.UnpicklingError, IndexError) def dumps(self, arg, protocol=None): return pickle.dumps(arg, protocol) @@ -119,6 +121,7 @@ if has_c_implementation: class CUnpicklerTests(PyUnpicklerTests): unpickler = _pickle.Unpickler + bad_stack_errors = (pickle.UnpicklingError,) class CPicklerTests(PyPicklerTests): pickler = _pickle.Pickler diff -r 59f33f4249ec -r b08c3a733fda Misc/NEWS --- a/Misc/NEWS Sun Nov 22 23:46:02 2015 -0800 +++ b/Misc/NEWS Mon Nov 23 15:20:21 2015 +0200 @@ -95,6 +95,8 @@ Library ------- +- Issue #23914: Fixed SystemError raised by unpickler on broken pickle data. + - Issue #25691: Fixed crash on deleting ElementTree.Element attributes. - Issue #25624: ZipFile now always writes a ZIP_STORED header for directory diff -r 59f33f4249ec -r b08c3a733fda Modules/_pickle.c --- a/Modules/_pickle.c Sun Nov 22 23:46:02 2015 -0800 +++ b/Modules/_pickle.c Mon Nov 23 15:20:21 2015 +0200 @@ -473,8 +473,8 @@ static PyObject * Pdata_pop(Pdata *self) { - PickleState *st = _Pickle_GetGlobalState(); if (Py_SIZE(self) == 0) { + PickleState *st = _Pickle_GetGlobalState(); PyErr_SetString(st->UnpicklingError, "bad pickle data"); return NULL; } @@ -5211,6 +5211,9 @@ if ((i = marker(self)) < 0) return -1; + if (Py_SIZE(self->stack) - i < 1) + return stack_underflow(); + args = Pdata_poptuple(self->stack, i + 1); if (args == NULL) return -1; @@ -5869,13 +5872,18 @@ static int load_append(UnpicklerObject *self) { + if (Py_SIZE(self->stack) - 1 <= 0) + return stack_underflow(); return do_append(self, Py_SIZE(self->stack) - 1); } static int load_appends(UnpicklerObject *self) { - return do_append(self, marker(self)); + Py_ssize_t i = marker(self); + if (i < 0) + return -1; + return do_append(self, i); } static int @@ -5925,7 +5933,10 @@ static int load_setitems(UnpicklerObject *self) { - return do_setitems(self, marker(self)); + Py_ssize_t i = marker(self); + if (i < 0) + return -1; + return do_setitems(self, i); } static int @@ -5935,6 +5946,8 @@ Py_ssize_t mark, len, i; mark = marker(self); + if (mark < 0) + return -1; len = Py_SIZE(self->stack); if (mark > len || mark <= 0) return stack_underflow();