marshal.loads() raises SystemError instead of ValueError on an invalid code object

[tonghuaroot]

Bug report

Bug description

marshal.loads() already reports corrupt input by raising ValueError
with a "bad marshal data (...)" message for essentially every malformed
case its reader can hit, for example:

• bad marshal data (unknown type code)
• bad marshal data (string size out of range)
• bad marshal data (tuple size out of range)

There are about a dozen such checks in Python/marshal.c.

One path is inconsistent with the rest. When the serialized data describes a
code object whose internal fields disagree with each other, r_object()
hands the fields to _PyCode_Validate(), which reports the inconsistency
with PyErr_BadInternalCall(). That surfaces to the caller as
SystemError: bad argument to internal function rather than the
ValueError used for all other corrupt marshal data.

A clear way to hit this is a code object whose localsplusnames tuple and
localspluskinds bytes have different lengths. Those two are parallel
(one kind byte per name), so they must be the same length, and
_PyCode_Validate() checks exactly that:

PyTuple_GET_SIZE(con->localsplusnames)
    != PyBytes_GET_SIZE(con->localspluskinds)

Reproduction

The script below marshals a real code object, then rewrites just the
localspluskinds record to be one byte shorter than the names tuple, so
the two lengths disagree:

import marshal, struct

# A tiny function: 3 localsplus slots (locals a, b, x). Their names live in
# a tuple, and there is a parallel "localspluskinds" bytes blob with one kind
# byte per name, so the two must have equal length.
src = "def f(a, b):\n    x = a + b\n    return x\n"
co = compile(src, "<repro>", "exec").co_consts[0]
n = len(co.co_varnames) + len(co.co_cellvars) + len(co.co_freevars)   # 3

blob = marshal.dumps(co)

# Locate the localspluskinds record: the TYPE_STRING ('s') whose payload is
# exactly n bytes long (one kind byte per name); the other 's' record here,
# the bytecode, is much longer.
i, payload = None, None
for off in range(len(blob) - 5):
    if blob[off] == ord('s'):
        if struct.unpack_from('<i', blob, off + 1)[0] == n:
            i, payload = off, blob[off + 5:off + 5 + n]
            break

# Drop one kind byte: names tuple still has length 3, kinds bytes now length 2.
shrunk = b's' + struct.pack('<i', n - 1) + payload[:n - 1]
corrupt = blob[:i] + shrunk + blob[i + 5 + n:]

marshal.loads(corrupt)

On current main this raises:

Traceback (most recent call last):
  File "repro.py", line 26, in <module>
    marshal.loads(corrupt)
    ~~~~~~~~~~~~~^^^^^^^^^
SystemError: Objects/codeobject.c:465: bad argument to internal function

SystemError is meant for genuine interpreter bugs, so getting one from
untrusted/corrupted marshal data is a consistency and robustness gap rather
than an interpreter fault.

Proposed fix

In the TYPE_CODE case of r_object(), when _PyCode_Validate() fails with
a SystemError, convert it to the same kind of error the rest of the reader
already uses:

ValueError: bad marshal data (invalid code object)

This is narrowly scoped: it only rewrites the SystemError raised by
PyErr_BadInternalCall() for an inconsistent code object, and leaves the
other _PyCode_Validate() failures (which already raise ValueError or
OverflowError) untouched.

I have a patch with a regression test and a NEWS entry ready, and would be
happy to open a PR.

CPython versions tested on

CPython main

Operating systems tested on

macOS

Linked PRs

• gh-151830: Raise ValueError instead of SystemError for invalid code objects in marshal.loads() #151831