From ca5b18c7703c04c73bea14ae37811e9786fd80c2 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 29 Jun 2026 02:11:03 +0000 Subject: [PATCH] [3.13] gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation (GH-151573) OrderedDict.copy() walks the internal linked list while building the new dict. The loop body can run arbitrary Python (a key's __eq__/__hash__, or a subclass __getitem__/__setitem__) which can clear the source dict and free the nodes being iterated. Detect this the same way OrderedDict.__eq__ already does (gh-119004): snapshot od_state before the loop, hold a strong reference to the key and read the hash before any reentrant call, and raise RuntimeError if the state changed before advancing to the next node. (cherry picked from commit 7d128e319f3730e776a9161a4b5e9de95c802eaf) --- Lib/test/test_ordered_dict.py | 33 +++++++++++++++++ ...-06-17-00-00-00.gh-issue-148660.odcopy.rst | 3 ++ Objects/odictobject.c | 36 +++++++++++++------ 3 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-06-17-00-00-00.gh-issue-148660.odcopy.rst diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index a9b6a84996e6590..4578efb00b1c436 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -874,6 +874,39 @@ def side_effect(self): self.assertDictEqual(dict1, dict.fromkeys((0, 4.2))) self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2))) + def test_issue148660_copy_clear_in_key_eq(self): + # gh-148660: od.copy() must not crash when a key's __eq__ clears od + # while copy() is inserting into the new dict. + armed = False + calls = 0 + class Key: + def __hash__(self): + return 1 + def __eq__(self, other): + nonlocal calls + if armed: + calls += 1 + if calls == 2: + od.clear() + return self is other + od = self.OrderedDict() + od[Key()] = "v1" + od[Key()] = "v2" + armed = True + msg = "OrderedDict mutated during iteration" + self.assertRaisesRegex(RuntimeError, msg, od.copy) + + def test_issue148660_copy_clear_in_subclass_getitem(self): + # gh-148660: od.copy() must not crash when a subclass __getitem__ + # clears od. + class OD(self.OrderedDict): + def __getitem__(self, key): + od.clear() + return "v" + od = OD([(1, "v1"), (2, "v2")]) + msg = "OrderedDict mutated during iteration" + self.assertRaisesRegex(RuntimeError, msg, od.copy) + @unittest.skipUnless(c_coll, 'requires the C version of the collections module') class CPythonOrderedDictTests(OrderedDictTests, diff --git a/Misc/NEWS.d/next/Library/2026-06-17-00-00-00.gh-issue-148660.odcopy.rst b/Misc/NEWS.d/next/Library/2026-06-17-00-00-00.gh-issue-148660.odcopy.rst new file mode 100644 index 000000000000000..30c2e366ca2ba07 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-17-00-00-00.gh-issue-148660.odcopy.rst @@ -0,0 +1,3 @@ +Fix a crash in :meth:`!collections.OrderedDict.copy` when a key's +``__eq__`` or a subclass method mutates the dict during the copy. Now +raises :exc:`RuntimeError` instead, as iteration does. diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 70fd9f5a0f65874..ec32cb7a361daaf 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1224,36 +1224,52 @@ odict_copy(register PyODictObject *od, PyObject *Py_UNUSED(ignored)) if (od_copy == NULL) return NULL; + /* The loop body may run arbitrary Python code which could mutate od and + free its nodes (gh-148660); detect that the same way __eq__ does. */ + size_t state = od->od_state; + if (PyODict_CheckExact(od)) { _odict_FOREACH(od, node) { - PyObject *key = _odictnode_KEY(node); - PyObject *value = _odictnode_VALUE(node, od); + PyObject *key = Py_NewRef(_odictnode_KEY(node)); + Py_hash_t hash = _odictnode_HASH(node); + PyObject *value = PyODict_GetItemWithError((PyObject *)od, key); if (value == NULL) { if (!PyErr_Occurred()) PyErr_SetObject(PyExc_KeyError, key); + Py_DECREF(key); goto fail; } - if (_PyODict_SetItem_KnownHash((PyObject *)od_copy, key, value, - _odictnode_HASH(node)) != 0) + int res = _PyODict_SetItem_KnownHash((PyObject *)od_copy, + key, value, hash); + Py_DECREF(key); + if (res != 0) goto fail; + if (od->od_state != state) + goto mutated; } } else { _odict_FOREACH(od, node) { - int res; - PyObject *value = PyObject_GetItem((PyObject *)od, - _odictnode_KEY(node)); - if (value == NULL) + PyObject *key = Py_NewRef(_odictnode_KEY(node)); + PyObject *value = PyObject_GetItem((PyObject *)od, key); + if (value == NULL) { + Py_DECREF(key); goto fail; - res = PyObject_SetItem((PyObject *)od_copy, - _odictnode_KEY(node), value); + } + int res = PyObject_SetItem((PyObject *)od_copy, key, value); Py_DECREF(value); + Py_DECREF(key); if (res != 0) goto fail; + if (od->od_state != state) + goto mutated; } } return od_copy; +mutated: + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); fail: Py_DECREF(od_copy); return NULL;