Issue #4176: Pickle would crash the interpreter when a __reduce__ function

does not return an iterator for the 4th and 5th items.
(sequence-like and mapping-like state)

Backport of r67049.

diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
index e1bc078154190975745df2a8a8f5c7605c683316..bf9bca78f98734d3a94ef31a02e0adffb55a825c 100644 (file)
--- a/Lib/test/pickletester.py
+++ b/Lib/test/pickletester.py
@@ -849,6 +849,29 @@ class AbstractPickleTests(unittest.TestCase):
             y = self.loads(s)
             self.assertEqual(y._reduce_called, 1)
 
+    def test_reduce_bad_iterator(self):
+        # Issue4176: crash when 4th and 5th items of __reduce__()
+        # are not iterators
+        class C(object):
+            def __reduce__(self):
+                # 4th item is not an iterator
+                return list, (), None, [], None
+        class D(object):
+            def __reduce__(self):
+                # 5th item is not an iterator
+                return dict, (), None, None, []
+
+        # Protocol 0 is less strict and also accept iterables.
+        for proto in 0, 1, 2:
+            try:
+                self.dumps(C(), proto)
+            except (AttributeError, pickle.PickleError, cPickle.PickleError):
+                pass
+            try:
+                self.dumps(D(), proto)
+            except (AttributeError, pickle.PickleError, cPickle.PickleError):
+                pass
+
 # Test classes for reduce_ex
 
 class REX_one(object):

diff --git a/Misc/NEWS b/Misc/NEWS
index e62bb7cf6b1737d75da10d02b11c73a387a5096d..6e8a25ad60fd11b2077ed4fe082cb48b382c3e78 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.5.3?
 Core and builtins
 -----------------
 
+- Issue #4176: Fixed a crash when pickling an object which ``__reduce__``
+  method does not return iterators for the 4th and 5th items.
+
 - Issue #3967: Fixed a crash in the count() and find() methods of string-like
   objects, when the "start" parameter is a huge value.
 

diff --git a/Modules/cPickle.c b/Modules/cPickle.c
index 537276c82cbad90b0a595c37784589a5f498bcf6..4842078de08c966f5821b02033a34a5bb3fa5aac 100644 (file)
--- a/Modules/cPickle.c
+++ b/Modules/cPickle.c
@@ -2141,13 +2141,14 @@ save_pers(Picklerobject *self, PyObject *args, PyObject *f)
  * appropriate __reduce__ method for ob.
  */
 static int
-save_reduce(Picklerobject *self, PyObject *args, PyObject *ob)
+save_reduce(Picklerobject *self, PyObject *args, PyObject *fn, PyObject *ob)
 {
        PyObject *callable;
        PyObject *argtup;
-        PyObject *state = NULL;
-        PyObject *listitems = NULL;
-        PyObject *dictitems = NULL;
+       PyObject *state = NULL;
+       PyObject *listitems = Py_None;
+       PyObject *dictitems = Py_None;
+       Py_ssize_t size;
 
        int use_newobj = self->proto >= 2;
 
@@ -2155,6 +2156,14 @@ save_reduce(Picklerobject *self, PyObject *args, PyObject *ob)
        static char build = BUILD;
        static char newobj = NEWOBJ;
 
+       size = PyTuple_Size(args);
+       if (size < 2 || size > 5) {
+               cPickle_ErrFormat(PicklingError, "tuple returned by "
+                       "%s must contain 2 through 5 elements",
+                       "O", fn);
+               return -1;
+       }
+
        if (! PyArg_UnpackTuple(args, "save_reduce", 2, 5,
                                &callable,
                                &argtup,
@@ -2164,17 +2173,32 @@ save_reduce(Picklerobject *self, PyObject *args, PyObject *ob)
                return -1;
 
        if (!PyTuple_Check(argtup)) {
-               PyErr_SetString(PicklingError,
-                               "args from reduce() should be a tuple");
+               cPickle_ErrFormat(PicklingError, "Second element of "
+                       "tuple returned by %s must be a tuple",
+                       "O", fn);
                return -1;
        }
 
        if (state == Py_None)
                state = NULL;
+
        if (listitems == Py_None)
                listitems = NULL;
+       else if (!PyIter_Check(listitems)) {
+               cPickle_ErrFormat(PicklingError, "Fourth element of "
+                       "tuple returned by %s must be an iterator, not %s",
+                       "Os", fn, listitems->ob_type->tp_name);
+               return -1;
+       }
+
        if (dictitems == Py_None)
                dictitems = NULL;
+       else if (!PyIter_Check(dictitems)) {
+               cPickle_ErrFormat(PicklingError, "Fifth element of "
+                       "tuple returned by %s must be an iterator, not %s",
+                       "Os", fn, dictitems->ob_type->tp_name);
+               return -1;
+       }
 
         /* Protocol 2 special case: if callable's name is __newobj__, use
          * NEWOBJ.  This consumes a lot of code.
@@ -2298,9 +2322,8 @@ save(Picklerobject *self, PyObject *args, int pers_save)
 {
        PyTypeObject *type;
        PyObject *py_ob_id = 0, *__reduce__ = 0, *t = 0;
-       PyObject *arg_tup;
        int res = -1;
-       int tmp, size;
+       int tmp;
 
         if (self->nesting++ > Py_GetRecursionLimit()){
                PyErr_SetString(PyExc_RuntimeError,
@@ -2527,30 +2550,14 @@ save(Picklerobject *self, PyObject *args, int pers_save)
                goto finally;
        }
 
-       if (! PyTuple_Check(t)) {
+       if (!PyTuple_Check(t)) {
                cPickle_ErrFormat(PicklingError, "Value returned by "
                                "%s must be string or tuple",
                                "O", __reduce__);
                goto finally;
        }
 
-       size = PyTuple_Size(t);
-       if (size < 2 || size > 5) {
-               cPickle_ErrFormat(PicklingError, "tuple returned by "
-                       "%s must contain 2 through 5 elements",
-                       "O", __reduce__);
-               goto finally;
-       }
-
-       arg_tup = PyTuple_GET_ITEM(t, 1);
-       if (!(PyTuple_Check(arg_tup) || arg_tup == Py_None))  {
-               cPickle_ErrFormat(PicklingError, "Second element of "
-                       "tuple returned by %s must be a tuple",
-                       "O", __reduce__);
-               goto finally;
-       }
-
-       res = save_reduce(self, t, args);
+       res = save_reduce(self, t, __reduce__, args);
 
   finally:
        self->nesting--;