gh-148653: Forbid marshalling recursive tuples (GH-155903)

Also fix a crash when unmarshalling a self-referencing tuple.

Co-authored-by: Michael Bommarito <michael.bommarito@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 3 people

Lib/test/test_marshal.py

@@ -342,14 +342,26 @@ def test_reference_loop_dict(self):
     def test_reference_loop_tuple(self):
         a = ([],)
         a[0].append(a)
-        for v in range(3):
+        for v in range(marshal.version + 1):
             self.assertRaises(ValueError, marshal.dumps, a, v)
+
+        a = ({},)
+        a[0][None] = a
+        for v in range(marshal.version + 1):
+            self.assertRaises(ValueError, marshal.dumps, a, v)
+
+    def test_shared_reference_tuple(self):
+        # A tuple referenced more than once still round-trips with the
+        # shared identity preserved.
+        a = (1, 2)
         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)
+            b = marshal.loads(marshal.dumps([a, a], v))
+            self.assertEqual(b[0], a)
+            self.assertIs(b[0], b[1])
+        big = tuple(range(300))  # too large for TYPE_SMALL_TUPLE
+        b = marshal.loads(marshal.dumps([big, big]))
+        self.assertEqual(b[0], big)
+        self.assertIs(b[0], b[1])
 
     def test_reference_loop_code(self):
         def f():
@@ -409,27 +421,6 @@ def test_loads_reference_loop_dict(self):
         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.
-        # This creates a reference loop which cannot be collected.
-        if False:
-            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}
@@ -439,6 +430,8 @@ def test_loads_abnormal_reference_loops(self):
 
         for data in [
             # Indirect self-references of immutable objects.
+            b'\xa8\x01\x00\x00\x00[\x01\x00\x00\x00r\x00\x00\x00\x00', # ([<R>],)
+            b'\xa8\x01\x00\x00\x00{Nr\x00\x00\x00\x000', # ({None: <R>},)
             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>])
@@ -449,12 +442,18 @@ def test_loads_abnormal_reference_loops(self):
             b'\xfdN{Nr\x00\x00\x00\x0000', # frozendict({None: {None: <R>})
 
             # Direct self-references which cannot be created in Python.
+            b'\xa8\x01\x00\x00\x00r\x00\x00\x00\x00', # (<R>,)
             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>)
+
+            # Indirect self-references which cannot be created in Python
+            # because of unhashability.
+            b'\xa8\x01\x00\x00\x00{r\x00\x00\x00\x00N0', # ({<R>: None},)
+            b'\xa8\x01\x00\x00\x00<\x01\x00\x00\x00r\x00\x00\x00\x00', # ({<R>},)
         ]:
             with self.subTest(data=data):
                 self.assertRaises(ValueError, marshal.loads, data)

Misc/NEWS.d/next/Core_and_Builtins/2026-08-16-19-33-01.gh-issue-148653.Kt3vQx.rst

@@ -0,0 +1,2 @@
+Forbid :mod:`marshalling <marshal>` recursive tuples, and fix a crash when
+unmarshalling a self-referencing tuple.

Python/marshal.c

@@ -417,7 +417,9 @@ w_ref(PyObject *v, char *flag, WFILE *p)
         }
         // Corresponding code should call w_complete() after
         // writing the object.
-        if (PyCode_Check(v) || PySlice_Check(v) || PyFrozenDict_CheckExact(v)) {
+        if (PyTuple_CheckExact(v) || PyCode_Check(v) || PySlice_Check(v) ||
+            PyFrozenDict_CheckExact(v))
+        {
             w |= 0x80000000LU;
         }
         if (_Py_hashtable_set(p->hashtable, Py_NewRef(v),
@@ -596,6 +598,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
         for (i = 0; i < n; i++) {
             w_object(PyTuple_GET_ITEM(v, i), p);
         }
+        w_complete(v, p);
     }
     else if (PyList_CheckExact(v)) {
         W_TYPE(TYPE_LIST, p);
@@ -1417,8 +1420,10 @@ r_object(RFILE *p)
             break;
         }
     _read_tuple:
+        idx = r_ref_reserve(flag, p);
+        if (idx < 0)
+            break;
         v = PyTuple_New(n);
-        R_REF(v);
         if (v == NULL)
             break;
 
@@ -1433,7 +1438,7 @@ r_object(RFILE *p)
             }
             PyTuple_SET_ITEM(v, i, v2);
         }
-        retval = v;
+        retval = r_ref_insert(v, idx, flag, p);
         break;
 
     case TYPE_LIST: