Skip to content

gh-151370: Fix marshal.dumps() crash on concurrent container mutation#151371

Open
tonghuaroot wants to merge 1 commit into
python:mainfrom
tonghuaroot:fix-marshal-uaf-clean
Open

gh-151370: Fix marshal.dumps() crash on concurrent container mutation#151371
tonghuaroot wants to merge 1 commit into
python:mainfrom
tonghuaroot:fix-marshal-uaf-clean

Conversation

@tonghuaroot

@tonghuaroot tonghuaroot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

w_complex_object() serializes a list, dict or set by handing each item
to w_object(). For an item that supports the buffer protocol this reaches
PyObject_GetBuffer(), which runs the item's __buffer__() (PEP 688) — pure
Python that can concurrently mutate, or drop the last reference to, the very
container being serialized. That crashes the interpreter in three places:

  • list — the loop caches n = PyList_GET_SIZE(v) and indexes with
    PyList_GET_ITEM(v, i), so shrinking the list reads past the end; and the
    buffer branch keeps using the item after PyObject_GetBuffer() returns, so
    freeing it is a use-after-free.
  • dictPyDict_Next() returns borrowed key/value; the key's
    __buffer__() can drop the dict's last reference to value, after which
    w_object(value) runs on freed memory.
  • set — serializing an element can clear the set (assert(i == n) fires,
    or a NULL slot reaches PyList_Sort) or grow it (writing past the pairs
    list that was pre-sized to the original length).

The container itself can also be freed: an item's __buffer__() may drop the
caller's last reference to the list/dict/set while w_complex_object()
is still iterating it.

The fix follows the bytes.join fix in gh-151295:

  • w_object() now keeps a reference to the object it is serializing across
    w_complex_object() (Py_INCREF/Py_DECREF), so neither an item nor the
    container can be freed by the Python that __buffer__() runs. The dict
    branch additionally keeps its borrowed key/value alive across the two
    w_object() calls.
  • After serializing each element, the list, dict and set branches
    recheck the container's size and report a change as RuntimeError instead of
    continuing from a now-stale index or iterator position; the set also bounds
    its write index against the pre-sized pairs list.

The size change is reported through marshal's existing error-code mechanism:
w_object() records a WFERR_* code and the top-level switch in
_PyMarshal_WriteObjectToString() maps it to an exception. That switch is
unconditional, so an exception set inside the loop would be overwritten;
producing the precise RuntimeError therefore requires a new code
(WFERR_CHANGED_SIZE) and a matching case, mirroring the codes already defined.

Same family as gh-151295 (bytes.join), fixed the same way.


This change was prepared with the assistance of an AI tool; I reviewed and
verified the fix and the regression test myself: a fresh main build with
before/after reproducers, the full test_marshal suite, and a refleak run.

…tation

An item's __buffer__() (PEP 688) runs Python while the list, dict or set is
being serialized, and that Python can mutate or free the container, causing a
use-after-free, an out-of-bounds access, or an abort.  Keep the serialized
object alive across w_complex_object() and report a size change as a
RuntimeError.

Same family as pythongh-151295 (bytes.join).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant