Skip to content

Commit 9dc1ae0

Browse files
authored
[3.13] gh-148660: Fix use-after-free in OrderedDict.copy() on reentrant mutation (GH-151573) (#152542)
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 7d128e3)
1 parent e041cdc commit 9dc1ae0

3 files changed

Lines changed: 62 additions & 10 deletions

File tree

Lib/test/test_ordered_dict.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,39 @@ def side_effect(self):
874874
self.assertDictEqual(dict1, dict.fromkeys((0, 4.2)))
875875
self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2)))
876876

877+
def test_issue148660_copy_clear_in_key_eq(self):
878+
# gh-148660: od.copy() must not crash when a key's __eq__ clears od
879+
# while copy() is inserting into the new dict.
880+
armed = False
881+
calls = 0
882+
class Key:
883+
def __hash__(self):
884+
return 1
885+
def __eq__(self, other):
886+
nonlocal calls
887+
if armed:
888+
calls += 1
889+
if calls == 2:
890+
od.clear()
891+
return self is other
892+
od = self.OrderedDict()
893+
od[Key()] = "v1"
894+
od[Key()] = "v2"
895+
armed = True
896+
msg = "OrderedDict mutated during iteration"
897+
self.assertRaisesRegex(RuntimeError, msg, od.copy)
898+
899+
def test_issue148660_copy_clear_in_subclass_getitem(self):
900+
# gh-148660: od.copy() must not crash when a subclass __getitem__
901+
# clears od.
902+
class OD(self.OrderedDict):
903+
def __getitem__(self, key):
904+
od.clear()
905+
return "v"
906+
od = OD([(1, "v1"), (2, "v2")])
907+
msg = "OrderedDict mutated during iteration"
908+
self.assertRaisesRegex(RuntimeError, msg, od.copy)
909+
877910

878911
@unittest.skipUnless(c_coll, 'requires the C version of the collections module')
879912
class CPythonOrderedDictTests(OrderedDictTests,
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :meth:`!collections.OrderedDict.copy` when a key's
2+
``__eq__`` or a subclass method mutates the dict during the copy. Now
3+
raises :exc:`RuntimeError` instead, as iteration does.

Objects/odictobject.c

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,36 +1224,52 @@ odict_copy(register PyODictObject *od, PyObject *Py_UNUSED(ignored))
12241224
if (od_copy == NULL)
12251225
return NULL;
12261226

1227+
/* The loop body may run arbitrary Python code which could mutate od and
1228+
free its nodes (gh-148660); detect that the same way __eq__ does. */
1229+
size_t state = od->od_state;
1230+
12271231
if (PyODict_CheckExact(od)) {
12281232
_odict_FOREACH(od, node) {
1229-
PyObject *key = _odictnode_KEY(node);
1230-
PyObject *value = _odictnode_VALUE(node, od);
1233+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1234+
Py_hash_t hash = _odictnode_HASH(node);
1235+
PyObject *value = PyODict_GetItemWithError((PyObject *)od, key);
12311236
if (value == NULL) {
12321237
if (!PyErr_Occurred())
12331238
PyErr_SetObject(PyExc_KeyError, key);
1239+
Py_DECREF(key);
12341240
goto fail;
12351241
}
1236-
if (_PyODict_SetItem_KnownHash((PyObject *)od_copy, key, value,
1237-
_odictnode_HASH(node)) != 0)
1242+
int res = _PyODict_SetItem_KnownHash((PyObject *)od_copy,
1243+
key, value, hash);
1244+
Py_DECREF(key);
1245+
if (res != 0)
12381246
goto fail;
1247+
if (od->od_state != state)
1248+
goto mutated;
12391249
}
12401250
}
12411251
else {
12421252
_odict_FOREACH(od, node) {
1243-
int res;
1244-
PyObject *value = PyObject_GetItem((PyObject *)od,
1245-
_odictnode_KEY(node));
1246-
if (value == NULL)
1253+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1254+
PyObject *value = PyObject_GetItem((PyObject *)od, key);
1255+
if (value == NULL) {
1256+
Py_DECREF(key);
12471257
goto fail;
1248-
res = PyObject_SetItem((PyObject *)od_copy,
1249-
_odictnode_KEY(node), value);
1258+
}
1259+
int res = PyObject_SetItem((PyObject *)od_copy, key, value);
12501260
Py_DECREF(value);
1261+
Py_DECREF(key);
12511262
if (res != 0)
12521263
goto fail;
1264+
if (od->od_state != state)
1265+
goto mutated;
12531266
}
12541267
}
12551268
return od_copy;
12561269

1270+
mutated:
1271+
PyErr_SetString(PyExc_RuntimeError,
1272+
"OrderedDict mutated during iteration");
12571273
fail:
12581274
Py_DECREF(od_copy);
12591275
return NULL;

0 commit comments

Comments
 (0)