]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.13] gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation...
authorGregory P. Smith <68491+gpshead@users.noreply.github.com>
Mon, 29 Jun 2026 02:56:52 +0000 (19:56 -0700)
committerGitHub <noreply@github.com>
Mon, 29 Jun 2026 02:56:52 +0000 (19:56 -0700)
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
Misc/NEWS.d/next/Library/2026-06-17-00-00-00.gh-issue-148660.odcopy.rst [new file with mode: 0644]
Objects/odictobject.c

index a9b6a84996e659012c7c4f5e3fed5b9669bc40bf..4578efb00b1c4365bdf2aebc1ce1a2c674ff27c4 100644 (file)
@@ -874,6 +874,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
@@ -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.
index 70fd9f5a0f658746f69ff8ffe4a0f55734085bc4..ec32cb7a361daafe6c5bbd047d4cb5905800d148 100644 (file)
@@ -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;