Skip to content

Commit 4847088

Browse files
committed
gh-145244: Fix use-after-free on borrowed dict key in json encoder
In encoder_encode_key_value(), key is a borrowed reference from PyDict_Next(). If the default callback mutates or clears the dict, key becomes a dangling pointer. The error path then calls _PyErr_FormatNote("%R", key) on freed memory. Fix by holding strong references to key and value unconditionally during encoding, not just in the free-threading build.
1 parent 1ac9d13 commit 4847088

File tree

3 files changed

+31
-7
lines changed

3 files changed

+31
-7
lines changed

Lib/test/test_json/test_dump.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,29 @@ def __lt__(self, o):
7777
d[1337] = "true.dat"
7878
self.assertEqual(self.dumps(d, sort_keys=True), '{"1337": "true.dat"}')
7979

80+
# gh-145244: UAF on borrowed key when default callback mutates dict
81+
def test_default_clears_dict_key_uaf(self):
82+
class Evil:
83+
pass
84+
85+
class AlsoEvil:
86+
pass
87+
88+
# Use a non-interned string key so it can actually be freed
89+
key = "A" * 100
90+
target = {key: Evil()}
91+
del key
92+
93+
def evil_default(obj):
94+
if isinstance(obj, Evil):
95+
target.clear()
96+
return AlsoEvil()
97+
raise TypeError("not serializable")
98+
99+
with self.assertRaises(TypeError):
100+
self.json.dumps(target, default=evil_default,
101+
check_circular=False)
102+
80103

81104
class TestPyDump(TestDump, PyTest): pass
82105

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fixed a use-after-free in :mod:`json` encoder when a ``default`` callback
2+
mutates the dictionary being serialized. Borrowed references to dictionary
3+
keys from ``PyDict_Next()`` are now properly held with strong references
4+
during encoding.

Modules/_json.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,24 +1774,21 @@ _encoder_iterate_dict_lock_held(PyEncoderObject *s, PyUnicodeWriter *writer,
17741774
PyObject *key, *value;
17751775
Py_ssize_t pos = 0;
17761776
while (PyDict_Next(dct, &pos, &key, &value)) {
1777-
#ifdef Py_GIL_DISABLED
1778-
// gh-119438: in the free-threading build the critical section on dct can get suspended
1777+
// gh-119438, gh-145244: key and value are borrowed refs from
1778+
// PyDict_Next(). encoder_encode_key_value() may invoke user
1779+
// Python code (the 'default' callback) that can mutate or
1780+
// clear the dict, so we must hold strong references.
17791781
Py_INCREF(key);
17801782
Py_INCREF(value);
1781-
#endif
17821783
if (encoder_encode_key_value(s, writer, first, dct, key, value,
17831784
indent_level, indent_cache,
17841785
separator) < 0) {
1785-
#ifdef Py_GIL_DISABLED
17861786
Py_DECREF(key);
17871787
Py_DECREF(value);
1788-
#endif
17891788
return -1;
17901789
}
1791-
#ifdef Py_GIL_DISABLED
17921790
Py_DECREF(key);
17931791
Py_DECREF(value);
1794-
#endif
17951792
}
17961793
return 0;
17971794
}

0 commit comments

Comments
 (0)