Three sites in the decoder decref an item again after PyList_SetItem() returns
-1. PyList_SetItem() steals the item reference even on failure (it calls
Py_XDECREF(item) internally before returning -1), so the extra Py_DECREF on
the error branch releases the same reference twice.
File: rapidjson.cpp
Functions: EndObject, EndArray
The three sites share one shape:
Py_ssize_t listLen = PyList_GET_SIZE(current.object);
rc = PyList_SetItem(current.object, listLen - 1, pair);
// NB: PyList_SetItem() steals a reference on the replacement, so it
// must not be DECREFed when the operation succeeds
if (rc == -1) {
Py_DECREF(pair); // double-free: PyList_SetItem already stole/released `pair`
return false;
}
Function
PyList_SetItem
erroneous Py_DECREF
item
| EndObject (key/value-pairs branch) |
rapidjson.cpp:1055 |
rapidjson.cpp:1061 |
pair |
| EndObject (array branch) |
rapidjson.cpp:1080 |
rapidjson.cpp:1086 |
replacement |
| EndArray (array branch) |
rapidjson.cpp:1175 |
rapidjson.cpp:1181 |
replacement |
The in-code comment is the tell: it says the item "must not be DECREFed when the
operation succeeds", implying the author believed a Py_DECREF is required on
failure. But PyList_SetItem() steals on failure too, so each item (refcount 1
at that point) is freed by the internal Py_XDECREF and then freed again by the
explicit Py_DECREF — a double-free / use-after-free.
The sibling dict branches at these sites use PyDict_SetItem() /
PyObject_SetItem(), which do not steal, and correctly Py_DECREF the item
afterward — confirming the list-branch handling is the odd one out.
Reachability
PyList_SetItem() returns -1 only when the first argument is not a list or the
index is out of range. In these branches current.object is always a list, and a
placeholder is normally appended (rapidjson.cpp:896, :912) before the matching
End* handler runs, so listLen - 1 is normally in range. The branch becomes
reachable if that placeholder is missing — e.g. an allocation-failure path where
PyList_GET_SIZE is 0, making listLen - 1 == -1 (out of range). See the
companion report on the unchecked PyList_Append() in Handle(), which can break
that invariant. This is an error-path defect, not something ordinary JSON input
triggers, but the handling is wrong and should be fixed independently.
Suggested fix
Drop the Py_DECREF on the error branch at all three sites:
rc = PyList_SetItem(current.object, listLen - 1, pair);
if (rc == -1) {
return false; /* PyList_SetItem already stole/released `pair` */
}
If a defensive check against an empty list is wanted, guard the index before
calling PyList_SetItem rather than decrefing afterward.
Three sites in the decoder decref an item again after PyList_SetItem() returns
-1. PyList_SetItem() steals the item reference even on failure (it calls
Py_XDECREF(item) internally before returning -1), so the extra Py_DECREF on
the error branch releases the same reference twice.
File: rapidjson.cpp
Functions: EndObject, EndArray
The three sites share one shape:
The in-code comment is the tell: it says the item "must not be DECREFed when the
operation succeeds", implying the author believed a Py_DECREF is required on
failure. But PyList_SetItem() steals on failure too, so each item (refcount 1
at that point) is freed by the internal Py_XDECREF and then freed again by the
explicit Py_DECREF — a double-free / use-after-free.
The sibling dict branches at these sites use PyDict_SetItem() /
PyObject_SetItem(), which do not steal, and correctly Py_DECREF the item
afterward — confirming the list-branch handling is the odd one out.
Reachability
PyList_SetItem() returns -1 only when the first argument is not a list or the
index is out of range. In these branches current.object is always a list, and a
placeholder is normally appended (rapidjson.cpp:896, :912) before the matching
End* handler runs, so listLen - 1 is normally in range. The branch becomes
reachable if that placeholder is missing — e.g. an allocation-failure path where
PyList_GET_SIZE is 0, making listLen - 1 == -1 (out of range). See the
companion report on the unchecked PyList_Append() in Handle(), which can break
that invariant. This is an error-path defect, not something ordinary JSON input
triggers, but the handling is wrong and should be fixed independently.
Suggested fix
Drop the Py_DECREF on the error branch at all three sites:
If a defensive check against an empty list is wanted, guard the index before
calling PyList_SetItem rather than decrefing afterward.