gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation (GH-151573)

* 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.

* gh-148660: fix NEWS nit, suppress undocumented OrderedDict.copy xref

diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py
index 642c2722711c7ca705faab18e8f8ca30cf4a7597..84bb3fdfbaaa6e7e09ba83676f3c72ad15e52848 100644 (file)
--- a/Lib/test/test_ordered_dict.py
+++ b/Lib/test/test_ordered_dict.py
@@ -879,6 +879,39 @@ class CPythonOrderedDictSideEffects:
         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 (file)
index 0000000..30c2e36
--- /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 14c1b02405822cde7cf8f19daeb7feb293ceb0aa..6d7e436ff00867afc5730948bb113acf4437f146 100644 (file)
--- 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;