Skip to content

Unchecked PyList_Append in Handle() array branch (use-after-free on failure) #232

Description

@K-ANOY

The array branch of Handle() ignores the return value of PyList_Append() and then decrefs the value unconditionally. On an append failure (e.g. allocation failure) this drops the last reference to a freshly created nested container that StartArray() / StartObject() are about to keep using, causing a use-after-free — and it also leaves the parser thinking the placeholder was inserted.

File: rapidjson.cpp

Function: Handle

Relevant code:

} else {
    PyList_Append(current.object, value);   /* return value ignored */
    Py_DECREF(value);
}

(rapidjson.cpp:911)

Callers create the container and hand it to Handle() as a placeholder before
recursing, e.g. StartArray():

PyObject* list = PyList_New(0);          /* refcount 1 */
if (list == NULL) return false;

if (!Handle(list)) return false;         /* appends placeholder, then Py_DECREF(list) */

HandlerContext ctx;
ctx.object = list;
Py_INCREF(list);                         /* uses `list` after Handle() */
stack.push_back(ctx);

On the success path, PyList_Append() adds a reference and the subsequent
Py_DECREF(value) balances the creation reference, so the container survives via
the parent list. But if PyList_Append() fails, no reference is added, so
Py_DECREF(value) drops the container to refcount 0 and frees it. Handle()
still returns true, and StartArray() (or StartObject()) then does
Py_INCREF(list) and stores the dangling pointer in the handler stack — a
use-after-free.

There is a second consequence: because the placeholder was never inserted, a
later EndObject() / EndArray() computes listLen - 1 against a list missing
the expected slot, which can reach the PyList_SetItem() error branch (see the
companion double-free report).

Suggested fix

Check the append result and propagate the failure:

} else {
    int rc = PyList_Append(current.object, value);
    Py_DECREF(value);
    if (rc == -1) {
        return false;
    }
}

Returning false here lets StartArray() / StartObject() bail out via their
existing if (!Handle(...)) return false; checks before they touch the freed
container.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions