gh-148653: Fix SIGSEGV in marshal.loads for self-referencing tuples

[mjbommar]

PR summary

Fix a deterministic SIGSEGV in marshal.loads() caused by TYPE_TUPLE, TYPE_LIST, TYPE_DICT, and TYPE_SET registering containers in p->refs via R_REF() before their slots are populated. A crafted payload with a TYPE_REF back-reference to the partial container can reach a hashing site (PySet_Add calling tuplehash), which calls PyObject_Hash(NULL) on unfilled slots.

The fix adopts the existing two-phase r_ref_reserve() / r_ref_insert() pattern already used by TYPE_FROZENSET, TYPE_CODE, and TYPE_SLICE. The Py_None placeholder in p->refs is detected by the existing TYPE_REF handler at line 1675, which raises ValueError("bad marshal data (invalid reference)") instead of crashing.

16-byte reproducer:

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

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

Originally filed as GHSA-m7gv-g5p9-9qqq; PSRT assessed as outside the security threat model (marshal.loads is documented as not secure against malicious data). Converting to public issue + PR per their guidance.

First time committer introduction

I use Python extensively in data science and legal tech. I found this while building an automated vulnerability scanner seeded from prior CPython CVE fix diffs (CVE-2018-20406, CVE-2022-48560). Happy to iterate on the patch.

AI Disclosure

Claude Code (Anthropic) assisted with grepping the codebase, drafting the byte-stream reproducer, and constructing the regression tests. I reviewed and understand all code changes. The fix follows the existing r_ref_reserve/r_ref_insert two-phase pattern already used by TYPE_FROZENSET/TYPE_CODE/TYPE_SLICE in the same file.

• Issue: Crash: marshal.loads SIGSEGV on self-referencing TYPE_TUPLE with FLAG_REF #148653

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[mjbommar]

Trivial self-reference is broken. Pushing fix in a moment

[mjbommar]

@serhiy-storchaka - should I force push to trigger a retest? There was a flakey upstream apt failure

[StanFromIreland]

I re-ran it.

[serhiy-storchaka]

Try to load b'\xa9\x01[\x01\x00\x00\x00r\x00\x00\x00\x00'. Currently it works, but with this PR it fails.

[mjbommar]

Try to load b'\xa9\x01[\x01\x00\x00\x00r\x00\x00\x00\x00'. Currently it works, but with this PR it fails.

Good catch! I found a few other weird cases that regress:

  - Direct self-referential tuple: t = (t,)
    b'\xa9\x01r\x00\x00\x00\x00'
  - Tuple through dict value: ({'x': t},)
    b'\xa9\x01\xfb\xda\x01xr\x00\x00\x00\x000'
  - Tuple through list with multiple refs back to the same tuple
    b'\xa9\x01[\x02\x00\x00\x00r\x00\x00\x00\x00r\x00\x00\x00\x00'
  - Non-tuple root, but inner tuple is still the back-ref target
    Shape: [( [t], )]
    b'\xdb\x01\x00\x00\x00\xa9\x01\xdb\x01\x00\x00\x00r\x01\x00\x00\x00'

1. @serhiy-storchaka , would you want me to improve the test cases in ./Lib/test/test_marshal.py to add these all or leave that as-is?

2. I assume you had already thought of this too, which was the point of your example, but perhaps the real solution is to avoid changing r_object() and add the guard in tuple_hash() instead. If so, would you go with ValueError or SystemError?

[serhiy-storchaka]

There are many other examples, it was just the simplest case. It seems you searched examples by some kind of fuzz generator (or just asked LLM?). You cannot be sure that that you will cover all cases. But they can be found by simple combinatorics. I'll add more tests.

Some of cases can be created in Python, others (like self-containing tuple) can only be artificially constructed marshal data. We should support the former, because they can occur in real program. We should prevent crashes and creation of inconsistent data structures when loading the latter.

I am not sure that changing tuple.__hash__() is a good solution. First, it will add some overhead in normal cases and can hide bugs in other parts. Second, we have similar issues with frozenset and frozendict which cannot be solved by that method. This may be a complex issue.

[mjbommar]

There are many other examples, it was just the simplest case. It seems you searched examples by some kind of fuzz generator (or just asked LLM?). You cannot be sure that that you will cover all cases. But they can be found by simple combinatorics. I'll add more tests.

I ran a simple test with itertools and typing feeding spawning sub-Pythons and then asked Opus 4.6 to expand from there. You're right about the scope of the issue.

Some of cases can be created in Python, others (like self-containing tuple) can only be artificially constructed marshal data. We should support the former, because they can occur in real program. We should prevent crashes and creation of inconsistent data structures when loading the latter.

I am not sure that changing tuple.__hash__() is a good solution. First, it will add some overhead in normal cases and can hide bugs in other parts. Second, we have similar issues with frozenset and frozendict which cannot be solved by that method. This may be a complex issue.

I'm beginning to appreciate your last sentence here...I think a proper fix for this would likely end up causing a performance impact.

If you want a PR to expand the test coverage so no one makes the same mistake I did, I'd be happy to submit it for now and defer the design questions to you all.

[mjbommar]

See #148700