Issue #23914: Fixed SystemError raised by unpickler on broken pickle data.

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.

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

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):

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?
 ============================

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);
 }