Crash: marshal.loads SIGSEGV on self-referencing TYPE_TUPLE with FLAG_REF

[mjbommar]

Bug report

Bug description:

Summary

marshal.loads() deterministically segfaults on a 16-byte structural
payload containing a TYPE_TUPLE | FLAG_REF whose elements include a
TYPE_REF back to the partial tuple itself.

The root cause is that R_REF() registers the tuple in p->refs
before its slots are populated. A nested TYPE_REF back-reference
then yields the partial tuple to a hashing site (PySet_Add), and
tuplehash calls PyObject_Hash(NULL) on the unfilled slot.

TYPE_FROZENSET, TYPE_CODE, and TYPE_SLICE already use the correct
two-phase pattern (r_ref_reserve / r_ref_insert) that avoids this.
TYPE_TUPLE, TYPE_LIST, TYPE_DICT, and TYPE_SET do not.

Originally filed as GHSA-m7gv-g5p9-9qqq. PSRT assessed this as outside
the security threat model since marshal.loads is documented as not secure
against malicious data. Converting to a public bug per their guidance.

Reproducer

import marshal
marshal.loads(b'\xa8\x02\x00\x00\x00N<\x01\x00\x00\x00r\x00\x00\x00\x00')

Byte stream:

• \xa8 = TYPE_TUPLE | FLAG_REF
• \x02\x00\x00\x00 = n = 2
• N = item[0] = TYPE_NONE
• <\x01\x00\x00\x00 = item[1] = TYPE_SET, n = 1
• r\x00\x00\x00\x00 = element = TYPE_REF(0), the partial outer tuple

Exit code 139 (SIGSEGV). faulthandler stack:

PySet_Add -> set_add_entry -> PyObject_Hash -> tuplehash -> NULL deref

Affected versions

Crashes on every version I tested: 3.9, 3.10, 3.11, 3.12, 3.13, 3.14.

Behavioral change after fix

With the two-phase pattern, the Py_None placeholder in p->refs
is detected by the existing TYPE_REF handler at marshal.c:1675:

if (v == Py_None) {
    PyErr_SetString(PyExc_ValueError, "bad marshal data (invalid reference)");
    break;
}

So the fix changes the behavior from SIGSEGV to
ValueError: bad marshal data (invalid reference).

Suggested fix

A fix with regression tests for tuple, list, set, and dict
self-reference payloads is at #148652

CPython versions tested on:

3.14

Operating systems tested on:

Linux

Linked PRs

• gh-148653: Fix SIGSEGV in marshal.loads for self-referencing tuples #148652
• gh-148653: Fix some marshal errors related to recursive immutable objects #148698
• gh-148653: refactored marshal for cycle safety and performance #148700
• [3.14] gh-148653: Fix some marshal errors related to recursive immutable objects (GH-148698) #148711
• [3.13] gh-148653: Fix some marshal errors related to recursive code objects (GH-148698) (GH-148711) #148713
• gh-148653: Fix reference leaks in test_marshal introduced in gh-148698 #148725
• [3.14] gh-148653: Fix reference leaks in test_marshal introduced in gh-148698 (GH-148725) #148727
• [3.13] gh-148653: Fix reference leaks in test_marshal introduced in gh-148698 (GH-148725) #148728
• gh-148653: Reject hashing incompletely initialized tuples #152321
• gh-148653: Forbid marshalling recursive tuples #155903

[serhiy-storchaka]

>>> import marshal
>>> a = ([],)
>>> a[0].append(a)
>>> a
([(...)],)
>>> marshal.dumps(a)
b'\xa9\x01[\x01\x00\x00\x00r\x00\x00\x00\x00'
>>> marshal.loads(marshal.dumps(a))
([(...)],)

With PR #148652 it fails.

[serhiy-storchaka]

I have found other bugs in marshal. For example, a reference loop containing slice can be successfully marshalled, but the produced data cannot be unmarshalled.

>>> import marshal
>>> a = slice([])
>>> a.stop.append(a)
>>> a
slice(None, [slice(None, [...], None)], None)
>>> marshal.dumps(a, 5)
b'\xbaN[\x01\x00\x00\x00r\x00\x00\x00\x00N'
>>> marshal.loads(marshal.dumps(a, 5))
Traceback (most recent call last):
  File "<python-input-5>", line 1, in <module>
    marshal.loads(marshal.dumps(a, 5))
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
ValueError: bad marshal data (invalid reference)

This issue is larger than anticipated, so I'll work on it.

[serhiy-storchaka]

The problem with frozendict:

>>> import marshal
>>> a = frozendict({None: []})
>>> a[None].append(a)
>>> a
frozendict({None: [frozendict({...})]})
>>> marshal.dumps(a)
b'\xfdN[\x01\x00\x00\x00r\x00\x00\x00\x000'
>>> marshal.loads(marshal.dumps(a))
frozendict({None: [{None: [...]}]})

(note that the nested dict is not a frozendict).

[mjbommar]

This issue is larger than anticipated, so I'll work on it.
Sounds good, @serhiy-storchaka.

You can see my other comments on #148652:
#148652 (comment)

I think this is unfortunately really a graph problem where we would ideally have a fast and easy case for acyclic digraphs and a safer but slower case for cycles. I am only beginning to understand the problem fully compared to you, but do you think a cycle check can be accurately implemented with low overhead?

[serhiy-storchaka]

#148698 adds multiple tests and fixes several bugs, except the original bug, with recursive tuples. After merging it we will continue with tuples.

[mjbommar]

Hi @serhiy-storchaka , I was just about to hit send on the experimental branch in #148700 when you sent this. Coincidence!

Please let me know what you think of the general design. I am 99% sure I can erase the performance regression at this point but still iterating on the ceiling.