From dc78cc6f7cea2040114a350aa9db835cc433ae05 Mon Sep 17 00:00:00 2001 From: Neal Norwitz Date: Wed, 16 May 2007 20:09:36 +0000 Subject: [PATCH] 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. --- Lib/test/test_marshal.py | 4 + Misc/ACKS | 1 + Misc/NEWS | 3 + Python/marshal.c | 225 +++++++++++++++++++++++++++------------ 4 files changed, 164 insertions(+), 69 deletions(-) diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index f87495bae2d..bfdd2747eff 100644 --- 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 7524baefc2d..58dab22d024 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -413,6 +413,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 8e8254d9060..67c4750c61d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,9 @@ What's New in Python 2.5.2c1? Library ------- +- Fix bug in marshal where bad data would cause a segfault due to + lack of an infinite recursion check. + - HTML-escape the plain traceback in cgitb's HTML output, to prevent the traceback inadvertently or maliciously closing the comment and injecting HTML into the error page. diff --git a/Python/marshal.c b/Python/marshal.c index 776836e5575..8faea4726d8 100644 --- 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;