Skip to content

Commit 2775028

Browse files
committed
gh-142734: fix OrderedDict copy heap-use-after-free security issue
1 parent 5989095 commit 2775028

File tree

3 files changed

+67
-6
lines changed

3 files changed

+67
-6
lines changed

Lib/test/test_ordered_dict.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import sys
1111
import unittest
1212
import weakref
13+
1314
from collections.abc import MutableMapping
1415
from test import mapping_tests, support
1516
from test.support import import_helper
@@ -729,6 +730,32 @@ def test_merge_operator(self):
729730
with self.assertRaises(ValueError):
730731
a |= "BAD"
731732

733+
def test_getitem_re_entrant_clear_during_copy(self):
734+
class Evil(self.OrderedDict):
735+
def __getitem__(self, key):
736+
super().clear()
737+
return None
738+
739+
evil_dict = Evil([(i, i) for i in range(4)])
740+
result = evil_dict.copy()
741+
742+
self.assertEqual(len(result), 4)
743+
744+
def test_getitem_re_entrant_modify_during_copy(self):
745+
class Modifier(self.OrderedDict):
746+
def __getitem__(self, key):
747+
self['new_key'] = 'new_value'
748+
return super().__getitem__(key)
749+
750+
original = Modifier([(1, 'one'), (2, 'two')])
751+
result = original.copy()
752+
753+
self.assertIn(1, result)
754+
self.assertIn(2, result)
755+
self.assertEqual(result[1], 'one')
756+
self.assertEqual(result[2], 'two')
757+
self.assertEqual(result["new_key"], "new_value")
758+
732759
@support.cpython_only
733760
def test_ordered_dict_items_result_gc(self):
734761
# bpo-42536: OrderedDict.items's tuple-reuse speed trick breaks the GC's
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fix ordereddict copy heap-use-after-free security issue

Objects/odictobject.c

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,18 +1266,51 @@ OrderedDict_copy_impl(PyObject *od)
12661266
}
12671267
}
12681268
else {
1269+
Py_ssize_t i, size = PyODict_Size((PyODictObject *)od);
1270+
PyObject **keys;
1271+
if (size < 0) {
1272+
goto fail;
1273+
}
1274+
1275+
if (size == 0) {
1276+
return od_copy;
1277+
}
1278+
1279+
keys = PyMem_Malloc(size * sizeof(PyObject *));
1280+
if (keys == NULL) {
1281+
PyErr_NoMemory();
1282+
goto fail;
1283+
}
1284+
1285+
i = 0;
12691286
_odict_FOREACH(od, node) {
1287+
keys[i] = _odictnode_KEY(node);
1288+
Py_INCREF(keys[i]);
1289+
i++;
1290+
}
1291+
1292+
for (i = 0; i < size; i++) {
12701293
int res;
1271-
PyObject *value = PyObject_GetItem((PyObject *)od,
1272-
_odictnode_KEY(node));
1273-
if (value == NULL)
1294+
PyObject *value = PyObject_GetItem((PyObject *)od, keys[i]);
1295+
if (value == NULL) {
1296+
for (Py_ssize_t j = 0; j < size; j++) {
1297+
Py_DECREF(keys[j]);
1298+
}
1299+
PyMem_Free(keys);
12741300
goto fail;
1275-
res = PyObject_SetItem((PyObject *)od_copy,
1276-
_odictnode_KEY(node), value);
1301+
}
1302+
res = PyObject_SetItem((PyObject *)od_copy, keys[i], value);
12771303
Py_DECREF(value);
1278-
if (res != 0)
1304+
Py_DECREF(keys[i]);
1305+
if (res != 0) {
1306+
for (; i < size; i++) {
1307+
Py_DECREF(keys[i]);
1308+
}
1309+
PyMem_Free(keys);
12791310
goto fail;
1311+
}
12801312
}
1313+
PyMem_Free(keys);
12811314
}
12821315
return od_copy;
12831316

0 commit comments

Comments
 (0)