Skip to content

Commit 8ce35c3

Browse files
committed
gh-152405: Fix crash of rich-comparing MappingProxyType objects
1 parent 95bfaff commit 8ce35c3

3 files changed

Lines changed: 88 additions & 3 deletions

File tree

Lib/test/test_types.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,14 +1392,19 @@ def __hash__(self):
13921392
self.assertEqual(hash(view), hash(mapping))
13931393

13941394
def test_richcompare(self):
1395-
mp1 = self.mappingproxy({'a': 1})
1395+
dt1 = {'a': 1}
1396+
mp1 = self.mappingproxy(dt1)
13961397
mp1_2 = self.mappingproxy({'a': 1})
13971398
mp2 = self.mappingproxy({'a': 2})
13981399

13991400
self.assertTrue(mp1 == mp1_2)
14001401
self.assertFalse(mp1 != mp1_2)
14011402
self.assertFalse(mp1 == mp2)
14021403
self.assertTrue(mp1 != mp2)
1404+
self.assertTrue(mp1 == dt1)
1405+
self.assertTrue(mp1_2 == dt1)
1406+
self.assertFalse(mp2 == dt1)
1407+
self.assertTrue(mp2 != dt1)
14031408

14041409
msg = "not supported between instances of 'mappingproxy' and 'mappingproxy'"
14051410

@@ -1411,6 +1416,67 @@ def test_richcompare(self):
14111416
mp2 >= mp2
14121417
with self.assertRaisesRegex(TypeError, msg):
14131418
mp1_2 <= mp1
1419+
with self.assertRaisesRegex(
1420+
TypeError,
1421+
"not supported between instances of 'mappingproxy' and 'dict'",
1422+
):
1423+
mp1_2 <= dt1
1424+
1425+
def test_richcompare_mapping(self):
1426+
class CustomMapping(collections.abc.Mapping):
1427+
def __init__(self, data):
1428+
self._data = data
1429+
1430+
def __getitem__(self, key):
1431+
return self._data[key]
1432+
1433+
def __iter__(self):
1434+
return iter(self._data)
1435+
1436+
def __len__(self):
1437+
return len(self._data)
1438+
1439+
def __contains__(self, item):
1440+
return item in self._data
1441+
1442+
dt1 = {'a': 1}
1443+
mp1 = self.mappingproxy(CustomMapping(dt1))
1444+
mp1_2 = self.mappingproxy(dt1)
1445+
mp2 = self.mappingproxy({'a': 2})
1446+
1447+
self.assertTrue(mp1 == mp1_2)
1448+
self.assertTrue(mp1 == dt1)
1449+
self.assertTrue(mp1_2 == dt1)
1450+
self.assertTrue(dt1 == mp1)
1451+
self.assertTrue(dt1 == mp1_2)
1452+
self.assertFalse(mp2 == dt1)
1453+
self.assertFalse(dt1 == mp2)
1454+
self.assertFalse(mp1 != dt1)
1455+
self.assertFalse(dt1 != mp1)
1456+
1457+
class Evil:
1458+
def __eq__(self, other):
1459+
return other
1460+
1461+
leaked = mp1 == Evil()
1462+
self.assertIs(type(leaked), dict)
1463+
1464+
def test_richcompare_evil(self):
1465+
# https://github.com/python/cpython/issues/152405
1466+
class Evil:
1467+
def __eq__(self, other):
1468+
return other
1469+
1470+
# exposes the internals of `MappingProxyType` via richcompare:
1471+
leaked = vars(list) == Evil()
1472+
self.assertIs(type(leaked), dict)
1473+
name = "__mappingproxy_crash_probe__"
1474+
leaked[name] = lambda self: "probe"
1475+
self.assertIn(name, leaked)
1476+
self.assertNotHasAttr(list, name) # it used to return `True`
1477+
del leaked[name]
1478+
self.assertNotIn(name, leaked)
1479+
self.assertNotHasAttr(list, name) # it used to crash
14141480

14151481

14161482
class ClassCreationTests(unittest.TestCase):
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fixes a crash, when modifing :class:`types.MappingProxyType` exposed via a
2+
rich-compare operation with :func:`vars` function. It was possible to mutate
3+
the internals of builtins types via this technique.
4+
5+
It changes the object that is passed to rich-compare methods
6+
from the real internal mapping type to be always :class:`dict`.

Objects/descrobject.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,9 +1232,22 @@ mappingproxy_traverse(PyObject *self, visitproc visit, void *arg)
12321232
static PyObject *
12331233
mappingproxy_richcompare(PyObject *self, PyObject *w, int op)
12341234
{
1235-
mappingproxyobject *v = (mappingproxyobject *)self;
12361235
if (op == Py_EQ || op == Py_NE) {
1237-
return PyObject_RichCompare(v->mapping, w, op);
1236+
mappingproxyobject *v = (mappingproxyobject *)self;
1237+
// We can't expose the `v->mapping` itself, so we create a dict copy:
1238+
// it was possible to mutate `v->mapping` in rich-compare methods.
1239+
// See gh-152405 on the details.
1240+
PyObject *copy = PyDict_New();
1241+
if (copy == NULL) {
1242+
return NULL;
1243+
}
1244+
if (PyDict_Update(copy, v->mapping)) {
1245+
Py_DECREF(copy);
1246+
return NULL;
1247+
}
1248+
PyObject *res = PyObject_RichCompare(copy, w, op);
1249+
Py_DECREF(copy);
1250+
return res;
12381251
}
12391252
Py_RETURN_NOTIMPLEMENTED;
12401253
}

0 commit comments

Comments
 (0)