From bf3bdde1848ebae35c3ada5c86cd52fd97e88b9e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 17 Jun 2026 04:25:40 +0000 Subject: [PATCH] gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation 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. --- 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 642c2722711c7c..84bb3fdfbaaa6e 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -879,6 +879,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 00000000000000..bfe5b1029797fd --- /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 14c1b02405822c..6d7e436ff00867 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1251,36 +1251,52 @@ OrderedDict_copy_impl(PyObject *od) 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 = _PyODictObject_CAST(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(od, key); if (value == NULL) { if (!PyErr_Occurred()) PyErr_SetObject(PyExc_KeyError, key); + Py_DECREF(key); goto fail; } - if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value, - _odictnode_HASH(node)) != 0) + int res = _PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, + key, value, hash); + Py_DECREF(key); + if (res != 0) goto fail; + if (_PyODictObject_CAST(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(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 (_PyODictObject_CAST(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;