Skip to content

Commit bf3bdde

Browse files
committed
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.
1 parent 9e863fa commit bf3bdde

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
@@ -879,6 +879,39 @@ def side_effect(self):
879879
self.assertDictEqual(dict1, dict.fromkeys((0, 4.2)))
880880
self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2)))
881881

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

883916
@unittest.skipUnless(c_coll, 'requires the C version of the collections module')
884917
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
@@ -1251,36 +1251,52 @@ OrderedDict_copy_impl(PyObject *od)
12511251
if (od_copy == NULL)
12521252
return NULL;
12531253

1254+
/* The loop body may run arbitrary Python code which could mutate od and
1255+
free its nodes (gh-148660); detect that the same way __eq__ does. */
1256+
size_t state = _PyODictObject_CAST(od)->od_state;
1257+
12541258
if (PyODict_CheckExact(od)) {
12551259
_odict_FOREACH(od, node) {
1256-
PyObject *key = _odictnode_KEY(node);
1257-
PyObject *value = _odictnode_VALUE(node, od);
1260+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1261+
Py_hash_t hash = _odictnode_HASH(node);
1262+
PyObject *value = PyODict_GetItemWithError(od, key);
12581263
if (value == NULL) {
12591264
if (!PyErr_Occurred())
12601265
PyErr_SetObject(PyExc_KeyError, key);
1266+
Py_DECREF(key);
12611267
goto fail;
12621268
}
1263-
if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value,
1264-
_odictnode_HASH(node)) != 0)
1269+
int res = _PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy,
1270+
key, value, hash);
1271+
Py_DECREF(key);
1272+
if (res != 0)
12651273
goto fail;
1274+
if (_PyODictObject_CAST(od)->od_state != state)
1275+
goto mutated;
12661276
}
12671277
}
12681278
else {
12691279
_odict_FOREACH(od, node) {
1270-
int res;
1271-
PyObject *value = PyObject_GetItem((PyObject *)od,
1272-
_odictnode_KEY(node));
1273-
if (value == NULL)
1280+
PyObject *key = Py_NewRef(_odictnode_KEY(node));
1281+
PyObject *value = PyObject_GetItem(od, key);
1282+
if (value == NULL) {
1283+
Py_DECREF(key);
12741284
goto fail;
1275-
res = PyObject_SetItem((PyObject *)od_copy,
1276-
_odictnode_KEY(node), value);
1285+
}
1286+
int res = PyObject_SetItem((PyObject *)od_copy, key, value);
12771287
Py_DECREF(value);
1288+
Py_DECREF(key);
12781289
if (res != 0)
12791290
goto fail;
1291+
if (_PyODictObject_CAST(od)->od_state != state)
1292+
goto mutated;
12801293
}
12811294
}
12821295
return od_copy;
12831296

1297+
mutated:
1298+
PyErr_SetString(PyExc_RuntimeError,
1299+
"OrderedDict mutated during iteration");
12841300
fail:
12851301
Py_DECREF(od_copy);
12861302
return NULL;

0 commit comments

Comments
 (0)