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

@@ -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,

Misc/ACKS

@@ -413,6 +413,7 @@ Dieter Maurer
 Greg McFarlane
 Michael McLay
 Gordon McMillan
+Damien Miller
 Jay T. Miller
 Chris McDonough
 Andrew McNamara

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.

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