Fix SIGSEGV in marshal.loads via self-referencing containers (GHSA-m7gv-g5p9-9qqq)

TYPE_TUPLE, TYPE_LIST, TYPE_DICT, and TYPE_SET used R_REF() to register
containers in p->refs immediately after allocation, before populating
their slots. A crafted payload containing a TYPE_REF back-reference to
the partial container could reach a hashing or iteration site with NULL
slots, causing tuplehash/PyObject_Hash(NULL) -> SIGSEGV.

Fix: use the existing two-phase r_ref_reserve()/r_ref_insert() pattern
(already used by TYPE_FROZENSET, TYPE_CODE, and TYPE_SLICE) for all
four container types. r_ref_reserve() places Py_None as a placeholder
in p->refs; the TYPE_REF handler (line 1675) already detects Py_None
and raises ValueError("bad marshal data (invalid reference)"). After
the container is fully populated, r_ref_insert() replaces the
placeholder with the real object.

Includes regression tests for tuple, list, set, and dict self-reference
payloads.

Author: mjbommar

Lib/test/test_marshal.py

@@ -731,5 +731,51 @@ def test_read_object_from_file(self):
             os_helper.unlink(os_helper.TESTFN)
 
 
+class SelfRefContainerTest(unittest.TestCase):
+    """Regression tests for containers with FLAG_REF back-references.
+
+    Before the fix, R_REF registered containers in p->refs before their
+    slots were populated. A TYPE_REF back-reference to the partial
+    container could then reach a hashing or iteration site with NULL
+    slots, crashing the interpreter (SIGSEGV).
+
+    The fix uses the two-phase r_ref_reserve/r_ref_insert pattern so the
+    Py_None placeholder is detected by the TYPE_REF handler, raising
+    ValueError("bad marshal data (invalid reference)") instead.
+    """
+
+    def test_self_ref_tuple(self):
+        # TYPE_TUPLE|FLAG_REF n=2; NONE; TYPE_SET n=1; TYPE_REF(0)
+        payload = (b'\xa8\x02\x00\x00\x00N'
+                   b'<\x01\x00\x00\x00r\x00\x00\x00\x00')
+        with self.assertRaises(ValueError):
+            marshal.loads(payload)
+
+    def test_self_ref_list(self):
+        # TYPE_LIST|FLAG_REF n=2; NONE; TYPE_SET n=1; TYPE_REF(0)
+        payload = (b'\xa9\x02\x00\x00\x00N'
+                   b'<\x01\x00\x00\x00r\x00\x00\x00\x00')
+        with self.assertRaises(ValueError):
+            marshal.loads(payload)
+
+    def test_self_ref_set(self):
+        # TYPE_SET|FLAG_REF n=1; TYPE_TUPLE|FLAG_REF n=1; TYPE_REF(0)
+        # The set element is a tuple that back-refs the set itself.
+        payload = (b'\xbc\x01\x00\x00\x00'
+                   b'\xa8\x01\x00\x00\x00'
+                   b'r\x00\x00\x00\x00')
+        with self.assertRaises(ValueError):
+            marshal.loads(payload)
+
+    def test_self_ref_dict(self):
+        # TYPE_DICT|FLAG_REF; key=NONE, val=TYPE_SET n=1 TYPE_REF(0); sentinel
+        payload = (b'\xfb'
+                   b'N'
+                   b'<\x01\x00\x00\x00r\x00\x00\x00\x00'
+                   b'\x00')
+        with self.assertRaises(ValueError):
+            marshal.loads(payload)
+
+
 if __name__ == "__main__":
     unittest.main()

Python/marshal.c

@@ -1385,7 +1385,9 @@ r_object(RFILE *p)
         }
     _read_tuple:
         v = PyTuple_New(n);
-        R_REF(v);
+        idx = r_ref_reserve(flag, p);
+        if (idx < 0)
+            Py_CLEAR(v);
         if (v == NULL)
             break;
 
@@ -1400,6 +1402,7 @@ r_object(RFILE *p)
             }
             PyTuple_SET_ITEM(v, i, v2);
         }
+        v = r_ref_insert(v, idx, flag, p);
         retval = v;
         break;
 
@@ -1413,7 +1416,9 @@ r_object(RFILE *p)
             break;
         }
         v = PyList_New(n);
-        R_REF(v);
+        idx = r_ref_reserve(flag, p);
+        if (idx < 0)
+            Py_CLEAR(v);
         if (v == NULL)
             break;
         for (i = 0; i < n; i++) {
@@ -1427,13 +1432,16 @@ r_object(RFILE *p)
             }
             PyList_SET_ITEM(v, i, v2);
         }
+        v = r_ref_insert(v, idx, flag, p);
         retval = v;
         break;
 
     case TYPE_DICT:
     case TYPE_FROZENDICT:
         v = PyDict_New();
-        R_REF(v);
+        idx = r_ref_reserve(flag, p);
+        if (idx < 0)
+            Py_CLEAR(v);
         if (v == NULL)
             break;
         for (;;) {
@@ -1466,6 +1474,7 @@ r_object(RFILE *p)
                 Py_CLEAR(v);
             }
         }
+        v = r_ref_insert(v, idx, flag, p);
         retval = v;
         break;
 
@@ -1490,12 +1499,7 @@ r_object(RFILE *p)
         }
         else {
             v = (type == TYPE_SET) ? PySet_New(NULL) : PyFrozenSet_New(NULL);
-            if (type == TYPE_SET) {
-                R_REF(v);
-            } else {
-                /* must use delayed registration of frozensets because they must
-                 * be init with a refcount of 1
-                 */
+            {
                 idx = r_ref_reserve(flag, p);
                 if (idx < 0)
                     Py_CLEAR(v); /* signal error */
@@ -1520,8 +1524,7 @@ r_object(RFILE *p)
                 }
                 Py_DECREF(v2);
             }
-            if (type != TYPE_SET)
-                v = r_ref_insert(v, idx, flag, p);
+            v = r_ref_insert(v, idx, flag, p);
             retval = v;
         }
         break;