gh-148653: Fix some marshal errors related to recursive immutable objects (GH-148698)

Forbid marshalling recursive code, slice and frozendict objects which
cannot be correctly unmarshalled.
Reject invalid marshal data produced by marshalling recursive frozendict
objects which was previously incorrectly unmarshalled.
Add multiple tests for recursive data structures.

Author: serhiy-storchaka

Lib/test/test_marshal.py

@@ -317,6 +317,133 @@ def test_recursion_limit(self):
         last.append([0])
         self.assertRaises(ValueError, marshal.dumps, head)
 
+    def test_reference_loop_list(self):
+        a = []
+        a.append(a)
+        for v in range(3):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+        for v in range(3, marshal.version + 1):
+            d = marshal.dumps(a, v)
+            b = marshal.loads(d)
+            self.assertIsInstance(b, list)
+            self.assertIs(b[0], b)
+
+    def test_reference_loop_dict(self):
+        a = {}
+        a[None] = a
+        for v in range(3):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+        for v in range(3, marshal.version + 1):
+            d = marshal.dumps(a, v)
+            b = marshal.loads(d)
+            self.assertIsInstance(b, dict)
+            self.assertIs(b[None], b)
+
+    def test_reference_loop_tuple(self):
+        a = ([],)
+        a[0].append(a)
+        for v in range(3):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+        for v in range(3, marshal.version + 1):
+            d = marshal.dumps(a, v)
+            b = marshal.loads(d)
+            self.assertIsInstance(b, tuple)
+            self.assertIsInstance(b[0], list)
+            self.assertIs(b[0][0], b)
+
+    def test_reference_loop_code(self):
+        def f():
+            return 1234.5
+        code = f.__code__
+        a = []
+        code = code.replace(co_consts=code.co_consts + (a,))
+        a.append(code)
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, code, v)
+
+    def test_reference_loop_slice(self):
+        a = slice([], None)
+        a.start.append(a)
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+
+        a = slice(None, [])
+        a.stop.append(a)
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+
+        a = slice(None, None, [])
+        a.step.append(a)
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+
+    def test_reference_loop_frozendict(self):
+        a = frozendict({None: []})
+        a[None].append(a)
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+
+    def test_loads_reference_loop_list(self):
+        data = b'\xdb\x01\x00\x00\x00r\x00\x00\x00\x00' # [<R>]
+        a = marshal.loads(data)
+        self.assertIsInstance(a, list)
+        self.assertIs(a[0], a)
+
+    def test_loads_reference_loop_dict(self):
+        data = b'\xfbNr\x00\x00\x00\x000' # {None: <R>}
+        a = marshal.loads(data)
+        self.assertIsInstance(a, dict)
+        self.assertIs(a[None], a)
+
+    def test_loads_abnormal_reference_loops(self):
+        # Indirect self-references of tuples.
+        data = b'\xa8\x01\x00\x00\x00[\x01\x00\x00\x00r\x00\x00\x00\x00' # ([<R>],)
+        a = marshal.loads(data)
+        self.assertIsInstance(a, tuple)
+        self.assertIsInstance(a[0], list)
+        self.assertIs(a[0][0], a)
+
+        data = b'\xa8\x01\x00\x00\x00{Nr\x00\x00\x00\x000' # ({None: <R>},)
+        a = marshal.loads(data)
+        self.assertIsInstance(a, tuple)
+        self.assertIsInstance(a[0], dict)
+        self.assertIs(a[0][None], a)
+
+        # Direct self-reference which cannot be created in Python.
+        data = b'\xa8\x01\x00\x00\x00r\x00\x00\x00\x00' # (<R>,)
+        a = marshal.loads(data)
+        self.assertIsInstance(a, tuple)
+        self.assertIs(a[0], a)
+
+        # Direct self-references which cannot be created in Python
+        # because of unhashability.
+        data = b'\xfbr\x00\x00\x00\x00N0' # {<R>: None}
+        self.assertRaises(TypeError, marshal.loads, data)
+        data = b'\xbc\x01\x00\x00\x00r\x00\x00\x00\x00' # {<R>}
+        self.assertRaises(TypeError, marshal.loads, data)
+
+        for data in [
+            # Indirect self-references of immutable objects.
+            b'\xba[\x01\x00\x00\x00r\x00\x00\x00\x00NN', # slice([<R>], None)
+            b'\xbaN[\x01\x00\x00\x00r\x00\x00\x00\x00N', # slice(None, [<R>])
+            b'\xbaNN[\x01\x00\x00\x00r\x00\x00\x00\x00', # slice(None, None, [<R>])
+            b'\xba{Nr\x00\x00\x00\x000NN', # slice({None: <R>}, None)
+            b'\xbaN{Nr\x00\x00\x00\x000N', # slice(None, {None: <R>})
+            b'\xbaNN{Nr\x00\x00\x00\x000', # slice(None, None, {None: <R>})
+            b'\xfdN[\x01\x00\x00\x00r\x00\x00\x00\x000', # frozendict({None: [<R>]})
+            b'\xfdN{Nr\x00\x00\x00\x0000', # frozendict({None: {None: <R>})
+
+            # Direct self-references which cannot be created in Python.
+            b'\xbe\x01\x00\x00\x00r\x00\x00\x00\x00', # frozenset({<R>})
+            b'\xfdNr\x00\x00\x00\x000', # frozendict({None: <R>})
+            b'\xfdr\x00\x00\x00\x00N0', # frozendict({<R>: None})
+            b'\xbar\x00\x00\x00\x00NN', # slice(<R>, None)
+            b'\xbaNr\x00\x00\x00\x00N', # slice(None, <R>)
+            b'\xbaNNr\x00\x00\x00\x00', # slice(None, None, <R>)
+        ]:
+            with self.subTest(data=data):
+                self.assertRaises(ValueError, marshal.loads, data)
+
     def test_exact_type_match(self):
         # Former bug:
         #   >>> class Int(int): pass

Misc/NEWS.d/next/Core_and_Builtins/2026-04-17-20-37-02.gh-issue-148653.nbbHMh.rst

@@ -0,0 +1,2 @@
+Forbid :mod:`marshalling <marshal>` recursive code objects, :class:`slice`
+and :class:`frozendict` objects which cannot be correctly unmarshalled.

Python/marshal.c

@@ -382,7 +382,6 @@ static int
 w_ref(PyObject *v, char *flag, WFILE *p)
 {
     _Py_hashtable_entry_t *entry;
-    int w;
 
     if (p->version < 3 || p->hashtable == NULL)
         return 0; /* not writing object references */
@@ -399,20 +398,28 @@ w_ref(PyObject *v, char *flag, WFILE *p)
     entry = _Py_hashtable_get_entry(p->hashtable, v);
     if (entry != NULL) {
         /* write the reference index to the stream */
-        w = (int)(uintptr_t)entry->value;
+        uintptr_t w = (uintptr_t)entry->value;
+        if (w & 0x80000000LU) {
+            PyErr_Format(PyExc_ValueError, "cannot marshal recursion %T objects", v);
+            goto err;
+        }
         /* we don't store "long" indices in the dict */
-        assert(0 <= w && w <= 0x7fffffff);
+        assert(w <= 0x7fffffff);
         w_byte(TYPE_REF, p);
-        w_long(w, p);
+        w_long((int)w, p);
         return 1;
     } else {
-        size_t s = p->hashtable->nentries;
+        size_t w = p->hashtable->nentries;
         /* we don't support long indices */
-        if (s >= 0x7fffffff) {
+        if (w >= 0x7fffffff) {
             PyErr_SetString(PyExc_ValueError, "too many objects");
             goto err;
         }
-        w = (int)s;
+        // Corresponding code should call w_complete() after
+        // writing the object.
+        if (PyCode_Check(v) || PySlice_Check(v) || PyFrozenDict_CheckExact(v)) {
+            w |= 0x80000000LU;
+        }
         if (_Py_hashtable_set(p->hashtable, Py_NewRef(v),
                               (void *)(uintptr_t)w) < 0) {
             Py_DECREF(v);
@@ -426,6 +433,27 @@ w_ref(PyObject *v, char *flag, WFILE *p)
     return 1;
 }
 
+static void
+w_complete(PyObject *v, WFILE *p)
+{
+    if (p->version < 3 || p->hashtable == NULL) {
+        return;
+    }
+    if (_PyObject_IsUniquelyReferenced(v)) {
+        return;
+    }
+
+    _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(p->hashtable, v);
+    if (entry == NULL) {
+        return;
+    }
+    assert(entry != NULL);
+    uintptr_t w = (uintptr_t)entry->value;
+    assert(w & 0x80000000LU);
+    w &= ~0x80000000LU;
+    entry->value = (void *)(uintptr_t)w;
+}
+
 static void
 w_complex_object(PyObject *v, char flag, WFILE *p);
 
@@ -599,6 +627,9 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
             w_object(value, p);
         }
         w_object((PyObject *)NULL, p);
+        if (PyFrozenDict_CheckExact(v)) {
+            w_complete(v, p);
+        }
     }
     else if (PyAnySet_CheckExact(v)) {
         PyObject *value;
@@ -684,6 +715,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
         w_object(co->co_linetable, p);
         w_object(co->co_exceptiontable, p);
         Py_DECREF(co_code);
+        w_complete(v, p);
     }
     else if (PyObject_CheckBuffer(v)) {
         /* Write unknown bytes-like objects as a bytes object */
@@ -709,6 +741,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
         w_object(slice->start, p);
         w_object(slice->stop, p);
         w_object(slice->step, p);
+        w_complete(v, p);
     }
     else {
         W_TYPE(TYPE_UNKNOWN, p);
@@ -1433,9 +1466,19 @@ r_object(RFILE *p)
     case TYPE_DICT:
     case TYPE_FROZENDICT:
         v = PyDict_New();
-        R_REF(v);
-        if (v == NULL)
+        if (v == NULL) {
             break;
+        }
+        if (type == TYPE_DICT) {
+            R_REF(v);
+        }
+        else {
+            idx = r_ref_reserve(flag, p);
+            if (idx < 0) {
+                Py_CLEAR(v);
+                break;
+            }
+        }
         for (;;) {
             PyObject *key, *val;
             key = r_object(p);
@@ -1458,13 +1501,7 @@ r_object(RFILE *p)
             Py_CLEAR(v);
         }
         if (type == TYPE_FROZENDICT && v != NULL) {
-            PyObject *frozendict = PyFrozenDict_New(v);
-            if (frozendict != NULL) {
-                Py_SETREF(v, frozendict);
-            }
-            else {
-                Py_CLEAR(v);
-            }
+            Py_SETREF(v, PyFrozenDict_New(v));
         }
         retval = v;
         break;