Fix bug in marshal where bad data would cause a segfault due to

lack of an infinite recursion check.

Contributed by Damien Miller at Google.

diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py
index f87495bae2dde27969b323105bf69f868e2423e1..bfdd2747effe70e8e4f224698fcfefd5e01e3b10 100644 (file)
--- a/Lib/test/test_marshal.py
+++ b/Lib/test/test_marshal.py
@@ -220,6 +220,10 @@ class BugsTestCase(unittest.TestCase):
             except Exception:
                 pass
 
+    def test_recursion(self):
+        s = 'c' + ('X' * 4*4) + '{' * 2**20
+        self.assertRaises(ValueError, marshal.loads, s)
+
 def test_main():
     test_support.run_unittest(IntTestCase,
                               FloatTestCase,

diff --git a/Misc/ACKS b/Misc/ACKS
index b47ec6e5887df50744c9975e6ff4b14c5cc1b5e4..b0a0372ae91bcbc7a71e87b302237c9af956750e 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -429,6 +429,7 @@ Dieter Maurer
 Greg McFarlane
 Michael McLay
 Gordon McMillan
+Damien Miller
 Jay T. Miller
 Chris McDonough
 Andrew McNamara

diff --git a/Misc/NEWS b/Misc/NEWS
index 1b3e1d970921d2d8236456c06646f8c82b6d92ac..eefa1816baf91440100f98946e6b11e6af2ec6da 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -207,6 +207,9 @@ Core and builtins
 Library
 -------
 
+- Fix bug in marshal where bad data would cause a segfault due to
+  lack of an infinite recursion check.
+
 - Removed plat-freebsd2 and plat-freebsd3 directories (and IN.py in
   the directories).
 

diff --git a/Python/marshal.c b/Python/marshal.c
index 776836e557574428e4855de0d1db9c14bb8fe5de..8faea4726d89f687108c5da51ab0d9e84efd4e4e 100644 (file)
--- a/Python/marshal.c
+++ b/Python/marshal.c
@@ -246,9 +246,16 @@ w_object(PyObject *v, WFILE *p)
                                goto exit;
                        }
                        else {
+                               int ok;
                                o = PyInt_FromSsize_t(PyDict_Size(p->strings));
-                               PyDict_SetItem(p->strings, v, o);
-                               Py_DECREF(o);
+                               ok = o &&
+                                    PyDict_SetItem(p->strings, v, o) >= 0;
+                               Py_XDECREF(o);
+                               if (!ok) {
+                                       p->depth--;
+                                       p->error = 1;
+                                       return;
+                               }
                                w_byte(TYPE_INTERNED, p);
                        }
                }
@@ -413,7 +420,7 @@ PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version)
 
 typedef WFILE RFILE; /* Same struct with different invariants */
 
-#define rs_byte(p) (((p)->ptr != (p)->end) ? (unsigned char)*(p)->ptr++ : EOF)
+#define rs_byte(p) (((p)->ptr < (p)->end) ? (unsigned char)*(p)->ptr++ : EOF)
 
 #define r_byte(p) ((p)->fp ? getc((p)->fp) : rs_byte(p))
 
@@ -504,42 +511,60 @@ r_object(RFILE *p)
        PyObject *v, *v2, *v3;
        long i, n;
        int type = r_byte(p);
+       PyObject *retval;
+
+       p->depth++;
+
+       if (p->depth > MAX_MARSHAL_STACK_DEPTH) {
+               p->depth--;
+               PyErr_SetString(PyExc_ValueError, "recursion limit exceeded");
+               return NULL;
+       }
 
        switch (type) {
 
        case EOF:
                PyErr_SetString(PyExc_EOFError,
                                "EOF read where object expected");
-               return NULL;
+               retval = NULL;
+               break;
 
        case TYPE_NULL:
-               return NULL;
+               retval = NULL;
+               break;
 
        case TYPE_NONE:
                Py_INCREF(Py_None);
-               return Py_None;
+               retval = Py_None;
+               break;
 
        case TYPE_STOPITER:
                Py_INCREF(PyExc_StopIteration);
-               return PyExc_StopIteration;
+               retval = PyExc_StopIteration;
+               break;
 
        case TYPE_ELLIPSIS:
                Py_INCREF(Py_Ellipsis);
-               return Py_Ellipsis;
+               retval = Py_Ellipsis;
+               break;
 
        case TYPE_FALSE:
                Py_INCREF(Py_False);
-               return Py_False;
+               retval = Py_False;
+               break;
 
        case TYPE_TRUE:
                Py_INCREF(Py_True);
-               return Py_True;
+               retval = Py_True;
+               break;
 
        case TYPE_INT:
-               return PyInt_FromLong(r_long(p));
+               retval = PyInt_FromLong(r_long(p));
+               break;
 
        case TYPE_INT64:
-               return r_long64(p);
+               retval = r_long64(p);
+               break;
 
        case TYPE_LONG:
                {
@@ -549,12 +574,15 @@ r_object(RFILE *p)
                        if (n < -INT_MAX || n > INT_MAX) {
                                PyErr_SetString(PyExc_ValueError,
                                                "bad marshal data");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        size = n<0 ? -n : n;
                        ob = _PyLong_New(size);
-                       if (ob == NULL)
-                               return NULL;
+                       if (ob == NULL) {
+                               retval = NULL;
+                               break;
+                       }
                        ob->ob_size = n;
                        for (i = 0; i < size; i++) {
                                int digit = r_short(p);
@@ -562,11 +590,14 @@ r_object(RFILE *p)
                                        Py_DECREF(ob);
                                        PyErr_SetString(PyExc_ValueError,
                                                        "bad marshal data");
-                                       return NULL;
+                                       ob = NULL;
+                                       break;
                                }
-                               ob->ob_digit[i] = digit;
+                               if (ob != NULL)
+                                       ob->ob_digit[i] = digit;
                        }
-                       return (PyObject *)ob;
+                       retval = (PyObject *)ob;
+                       break;
                }
 
        case TYPE_FLOAT:
@@ -577,13 +608,16 @@ r_object(RFILE *p)
                        if (n == EOF || r_string(buf, (int)n, p) != n) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        buf[n] = '\0';
-                       PyFPE_START_PROTECT("atof", return 0)
+                       retval = NULL;
+                       PyFPE_START_PROTECT("atof", break)
                        dx = PyOS_ascii_atof(buf);
                        PyFPE_END_PROTECT(dx)
-                       return PyFloat_FromDouble(dx);
+                       retval = PyFloat_FromDouble(dx);
+                       break;
                }
 
        case TYPE_BINARY_FLOAT:
@@ -593,13 +627,16 @@ r_object(RFILE *p)
                        if (r_string((char*)buf, 8, p) != 8) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        x = _PyFloat_Unpack8(buf, 1);
                        if (x == -1.0 && PyErr_Occurred()) {
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
-                       return PyFloat_FromDouble(x);
+                       retval = PyFloat_FromDouble(x);
+                       break;
                }
 
 #ifndef WITHOUT_COMPLEX
@@ -611,23 +648,27 @@ r_object(RFILE *p)
                        if (n == EOF || r_string(buf, (int)n, p) != n) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        buf[n] = '\0';
-                       PyFPE_START_PROTECT("atof", return 0)
+                       retval = NULL;
+                       PyFPE_START_PROTECT("atof", break;)
                        c.real = PyOS_ascii_atof(buf);
                        PyFPE_END_PROTECT(c)
                        n = r_byte(p);
                        if (n == EOF || r_string(buf, (int)n, p) != n) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        buf[n] = '\0';
-                       PyFPE_START_PROTECT("atof", return 0)
+                       PyFPE_START_PROTECT("atof", break)
                        c.imag = PyOS_ascii_atof(buf);
                        PyFPE_END_PROTECT(c)
-                       return PyComplex_FromCComplex(c);
+                       retval = PyComplex_FromCComplex(c);
+                       break;
                }
 
        case TYPE_BINARY_COMPLEX:
@@ -637,22 +678,27 @@ r_object(RFILE *p)
                        if (r_string((char*)buf, 8, p) != 8) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        c.real = _PyFloat_Unpack8(buf, 1);
                        if (c.real == -1.0 && PyErr_Occurred()) {
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        if (r_string((char*)buf, 8, p) != 8) {
                                PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
                        c.imag = _PyFloat_Unpack8(buf, 1);
                        if (c.imag == -1.0 && PyErr_Occurred()) {
-                               return NULL;
+                               retval = NULL;
+                               break;
                        }
-                       return PyComplex_FromCComplex(c);
+                       retval = PyComplex_FromCComplex(c);
+                       break;
                }
 #endif
 
@@ -661,32 +707,42 @@ r_object(RFILE *p)
                n = r_long(p);
                if (n < 0 || n > INT_MAX) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyString_FromStringAndSize((char *)NULL, n);
-               if (v == NULL)
-                       return v;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                if (r_string(PyString_AS_STRING(v), (int)n, p) != n) {
                        Py_DECREF(v);
                        PyErr_SetString(PyExc_EOFError,
                                        "EOF read where object expected");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                if (type == TYPE_INTERNED) {
                        PyString_InternInPlace(&v);
-                       PyList_Append(p->strings, v);
+                       if (PyList_Append(p->strings, v) < 0) {
+                               retval = NULL;
+                               break;
+                       }
                }
-               return v;
+               retval = v;
+               break;
 
        case TYPE_STRINGREF:
                n = r_long(p);
                if (n < 0 || n >= PyList_GET_SIZE(p->strings)) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyList_GET_ITEM(p->strings, n);
                Py_INCREF(v);
-               return v;
+               retval = v;
+               break;
 
 #ifdef Py_USING_UNICODE
        case TYPE_UNICODE:
@@ -696,20 +752,25 @@ r_object(RFILE *p)
                n = r_long(p);
                if (n < 0 || n > INT_MAX) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                buffer = PyMem_NEW(char, n);
-               if (buffer == NULL)
-                       return PyErr_NoMemory();
+               if (buffer == NULL) {
+                       retval = PyErr_NoMemory();
+                       break;
+               }
                if (r_string(buffer, (int)n, p) != n) {
                        PyMem_DEL(buffer);
                        PyErr_SetString(PyExc_EOFError,
                                "EOF read where object expected");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyUnicode_DecodeUTF8(buffer, n, NULL);
                PyMem_DEL(buffer);
-               return v;
+               retval = v;
+               break;
            }
 #endif
 
@@ -717,11 +778,14 @@ r_object(RFILE *p)
                n = r_long(p);
                if (n < 0 || n > INT_MAX) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyTuple_New((int)n);
-               if (v == NULL)
-                       return v;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                for (i = 0; i < n; i++) {
                        v2 = r_object(p);
                        if ( v2 == NULL ) {
@@ -734,17 +798,21 @@ r_object(RFILE *p)
                        }
                        PyTuple_SET_ITEM(v, (int)i, v2);
                }
-               return v;
+               retval = v;
+               break;
 
        case TYPE_LIST:
                n = r_long(p);
                if (n < 0 || n > INT_MAX) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyList_New((int)n);
-               if (v == NULL)
-                       return v;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                for (i = 0; i < n; i++) {
                        v2 = r_object(p);
                        if ( v2 == NULL ) {
@@ -755,14 +823,17 @@ r_object(RFILE *p)
                                v = NULL;
                                break;
                        }
-                       PyList_SetItem(v, (int)i, v2);
+                       PyList_SET_ITEM(v, (int)i, v2);
                }
-               return v;
+               retval = v;
+               break;
 
        case TYPE_DICT:
                v = PyDict_New();
-               if (v == NULL)
-                       return NULL;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                for (;;) {
                        PyObject *key, *val;
                        key = r_object(p);
@@ -778,18 +849,22 @@ r_object(RFILE *p)
                        Py_DECREF(v);
                        v = NULL;
                }
-               return v;
+               retval = v;
+               break;
 
        case TYPE_SET:
        case TYPE_FROZENSET:
                n = r_long(p);
-               if (n < 0) {
+               if (n < 0 || n > INT_MAX) {
                        PyErr_SetString(PyExc_ValueError, "bad marshal data");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                v = PyTuple_New((int)n);
-               if (v == NULL)
-                       return v;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                for (i = 0; i < n; i++) {
                        v2 = r_object(p);
                        if ( v2 == NULL ) {
@@ -802,21 +877,25 @@ r_object(RFILE *p)
                        }
                        PyTuple_SET_ITEM(v, (int)i, v2);
                }
-               if (v == NULL)
-                       return v;
+               if (v == NULL) {
+                       retval = NULL;
+                       break;
+               }
                if (type == TYPE_SET)
                        v3 = PySet_New(v);
                else
                        v3 = PyFrozenSet_New(v);
                Py_DECREF(v);
-               return v3;
+               retval = v3;
+               break;
 
        case TYPE_CODE:
                if (PyEval_GetRestricted()) {
                        PyErr_SetString(PyExc_RuntimeError,
                                "cannot unmarshal code objects in "
                                "restricted execution mode");
-                       return NULL;
+                       retval = NULL;
+                       break;
                }
                else {
                        int argcount;
@@ -888,15 +967,19 @@ r_object(RFILE *p)
                        Py_XDECREF(lnotab);
 
                }
-               return v;
+               retval = v;
+               break;
 
        default:
                /* Bogus data got written, which isn't ideal.
                   This will let you keep working and recover. */
                PyErr_SetString(PyExc_ValueError, "bad marshal data");
-               return NULL;
+               retval = NULL;
+               break;
 
        }
+       p->depth--;
+       return retval;
 }
 
 static PyObject *
@@ -1002,6 +1085,7 @@ PyMarshal_ReadObjectFromFile(FILE *fp)
        PyObject *result;
        rf.fp = fp;
        rf.strings = PyList_New(0);
+       rf.depth = 0;
        result = r_object(&rf);
        Py_DECREF(rf.strings);
        return result;
@@ -1016,6 +1100,7 @@ PyMarshal_ReadObjectFromString(char *str, Py_ssize_t len)
        rf.ptr = str;
        rf.end = str + len;
        rf.strings = PyList_New(0);
+       rf.depth = 0;
        result = r_object(&rf);
        Py_DECREF(rf.strings);
        return result;
@@ -1104,6 +1189,7 @@ marshal_load(PyObject *self, PyObject *f)
        }
        rf.fp = PyFile_AsFile(f);
        rf.strings = PyList_New(0);
+       rf.depth = 0;
        result = read_object(&rf);
        Py_DECREF(rf.strings);
        return result;
@@ -1132,6 +1218,7 @@ marshal_loads(PyObject *self, PyObject *args)
        rf.ptr = s;
        rf.end = s + n;
        rf.strings = PyList_New(0);
+       rf.depth = 0;
        result = read_object(&rf);
        Py_DECREF(rf.strings);
        return result;