From: Serhiy Storchaka Date: Mon, 23 Nov 2015 13:20:43 +0000 (+0200) Subject: Issue #23914: Fixed SystemError raised by unpickler on broken pickle data. X-Git-Tag: v2.7.12rc1~379 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5c137669e32e84744328cd8a1f9de4efad5281d0;p=thirdparty%2FPython%2Fcpython.git Issue #23914: Fixed SystemError raised by unpickler on broken pickle data. --- diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index d8346ea757b6..b0a3b041e391 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -7,7 +7,7 @@ import cStringIO import pickletools import copy_reg -from test.test_support import TestFailed, verbose, have_unicode, TESTFN +from test.test_support import TestFailed, verbose, have_unicode, TESTFN, captured_stdout try: from test.test_support import _2G, _1M, precisionbigmemtest except ImportError: @@ -634,6 +634,78 @@ class AbstractUnpickleTests(unittest.TestCase): self.assertEqual(unpickled, ([],)*2) self.assertIs(unpickled[0], unpickled[1]) + 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'i__builtin__\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.', + ] + for p in badpickles: + try: + self.assertRaises(self.bad_stack_errors, self.loads, p) + except: + print '***', repr(p) + raise + + def test_bad_mark(self): + badpickles = [ + b'c__builtin__\nlist\n)(R.', # REDUCE + b'c__builtin__\nlist\n()R.', + b']N(a.', # APPEND + b'cexceptions\nValueError\n)R}(b.', # BUILD + b'cexceptions\nValueError\n)R(}b.', + b'(Nd.', # DICT + b'}NN(s.', # SETITEM + b'}N(Ns.', + b'c__builtin__\nlist\n)(\x81.', # NEWOBJ + b'c__builtin__\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.', + ] + for p in badpickles: + # PyUnpickler prints reduce errors to stdout + try: + self.loads(p) + except (IndexError, AttributeError, TypeError, + pickle.UnpicklingError): + pass + class AbstractPickleTests(unittest.TestCase): # Subclass must define self.dumps, self.loads. diff --git a/Lib/test/test_cpickle.py b/Lib/test/test_cpickle.py index 0a1eb43a31a7..fff2b8052f8c 100644 --- a/Lib/test/test_cpickle.py +++ b/Lib/test/test_cpickle.py @@ -51,6 +51,7 @@ class cPickleTests(AbstractUnpickleTests, AbstractPickleTests, error = cPickle.BadPickleGet module = cPickle + bad_stack_errors = (cPickle.UnpicklingError,) class cPickleUnpicklerTests(AbstractUnpickleTests): @@ -63,6 +64,7 @@ class cPickleUnpicklerTests(AbstractUnpickleTests): self.close(f) error = cPickle.BadPickleGet + bad_stack_errors = (cPickle.UnpicklingError,) class cStringIOCUnpicklerTests(cStringIOMixin, cPickleUnpicklerTests): pass diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 2db75898e332..cbcf4912df6d 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -23,17 +23,18 @@ class PickleTests(AbstractUnpickleTests, AbstractPickleTests, module = pickle error = KeyError + bad_stack_errors = (IndexError,) class UnpicklerTests(AbstractUnpickleTests): error = KeyError + bad_stack_errors = (IndexError,) def loads(self, buf): f = StringIO(buf) u = pickle.Unpickler(f) return u.load() - class PicklerTests(AbstractPickleTests): def dumps(self, arg, proto=0, fast=0): diff --git a/Misc/NEWS b/Misc/NEWS index 6192be65fc0e..ec5c37831467 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -13,6 +13,7 @@ Core and Builtins Library ------- +- Issue #23914: Fixed SystemError raised by CPickle unpickler on broken data. What's New in Python 2.7.11? ============================ diff --git a/Modules/cPickle.c b/Modules/cPickle.c index 0e9372360f34..b053aa5d3a8b 100644 --- a/Modules/cPickle.c +++ b/Modules/cPickle.c @@ -3945,6 +3945,10 @@ load_obj(Unpicklerobject *self) Py_ssize_t i; if ((i = marker(self)) < 0) return -1; + + if (self->stack->length - i < 1) + return stackUnderflow(); + if (!( tup=Pdata_popTuple(self->stack, i+1))) return -1; PDATA_POP(self->stack, class); if (class) { @@ -4496,6 +4500,8 @@ do_append(Unpicklerobject *self, Py_ssize_t x) static int load_append(Unpicklerobject *self) { + if (self->stack->length - 1 <= 0) + return stackUnderflow(); return do_append(self, self->stack->length - 1); } @@ -4503,7 +4509,10 @@ load_append(Unpicklerobject *self) 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); } @@ -4515,6 +4524,14 @@ do_setitems(Unpicklerobject *self, Py_ssize_t x) if (!( (len=self->stack->length) >= x && x > 0 )) return stackUnderflow(); + if (len == x) /* nothing to do */ + return 0; + if ((len - x) % 2 != 0) { + /* Currupt or hostile pickle -- we never write one like this. */ + PyErr_SetString(UnpicklingError, + "odd number of items for SETITEMS"); + return -1; + } dict=self->stack->data[x-1]; @@ -4542,7 +4559,10 @@ load_setitem(Unpicklerobject *self) 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); }