Skip to content

Commit 8cdf960

Browse files
committed
gh-152405: Avoid exposing mappingproxy backing mapping
1 parent a69d0fc commit 8cdf960

3 files changed

Lines changed: 136 additions & 6 deletions

File tree

Lib/test/test_types.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,28 @@ def test_instantiation(self):
11801180
class MappingProxyTests(unittest.TestCase):
11811181
mappingproxy = types.MappingProxyType
11821182

1183+
class MappingWithoutCopy(collections.abc.Mapping):
1184+
def __init__(self, mapping):
1185+
self.mapping = mapping
1186+
1187+
def __getitem__(self, key):
1188+
return self.mapping[key]
1189+
1190+
def __iter__(self):
1191+
return iter(self.mapping)
1192+
1193+
def __len__(self):
1194+
return len(self.mapping)
1195+
1196+
class RecordingOperand:
1197+
def __eq__(self, other):
1198+
self.other = other
1199+
return None
1200+
1201+
def __ror__(self, other):
1202+
self.other = other
1203+
return None
1204+
11831205
def test_constructor(self):
11841206
class userdict(dict):
11851207
pass
@@ -1381,6 +1403,70 @@ def test_union(self):
13811403
self.assertDictEqual(mapping, {'a': 0, 'b': 1, 'c': 2})
13821404
self.assertDictEqual(other, {'c': 3, 'p': 0})
13831405

1406+
def test_richcompare_does_not_expose_mapping(self):
1407+
mapping = {}
1408+
view = self.mappingproxy(mapping)
1409+
1410+
class Sneaky:
1411+
def __eq__(self, other):
1412+
other['x'] = 42
1413+
return None
1414+
1415+
self.assertIsNone(view == Sneaky())
1416+
self.assertEqual(mapping, {})
1417+
1418+
other = self.RecordingOperand()
1419+
view = self.mappingproxy(self.MappingWithoutCopy(mapping))
1420+
self.assertIsNone(view == other)
1421+
self.assertIs(type(other.other), self.mappingproxy)
1422+
1423+
def test_union_does_not_expose_mapping(self):
1424+
mapping = {}
1425+
view = self.mappingproxy(mapping)
1426+
mappingproxy = self.mappingproxy
1427+
1428+
class SneakyRor:
1429+
def __ror__(self, other):
1430+
other['x'] = 42
1431+
return None
1432+
1433+
class SneakyOr:
1434+
def __or__(self, other):
1435+
if type(other) is mappingproxy:
1436+
return NotImplemented
1437+
other['y'] = 42
1438+
return None
1439+
1440+
self.assertIsNone(view | SneakyRor())
1441+
self.assertEqual(mapping, {})
1442+
self.assertIsNone(SneakyOr() | view)
1443+
self.assertEqual(mapping, {})
1444+
1445+
other = self.RecordingOperand()
1446+
view = self.mappingproxy(self.MappingWithoutCopy(mapping))
1447+
self.assertIsNone(view | other)
1448+
self.assertIs(type(other.other), self.mappingproxy)
1449+
1450+
def test_operator_dispatch_does_not_expose_type_dict(self):
1451+
code = textwrap.dedent("""
1452+
class SneakyEq:
1453+
def __eq__(self, other):
1454+
other['__mappingproxy_test_eq__'] = lambda self: None
1455+
return None
1456+
1457+
class SneakyRor:
1458+
def __ror__(self, other):
1459+
other['__mappingproxy_test_or__'] = lambda self: None
1460+
return None
1461+
1462+
vars(list) == SneakyEq()
1463+
vars(dict) | SneakyRor()
1464+
1465+
assert not hasattr(list, '__mappingproxy_test_eq__')
1466+
assert not hasattr(dict, '__mappingproxy_test_or__')
1467+
""")
1468+
assert_python_ok("-c", code)
1469+
13841470
def test_hash(self):
13851471
class HashableDict(dict):
13861472
def __hash__(self):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Prevent ``mappingproxy`` operator dispatch from exposing the underlying mapping
2+
during rich comparison and union operations. This fixes a case where reflected
3+
operator methods could access and mutate internal dictionaries, including those
4+
of built-in types.

Objects/descrobject.c

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,16 +1061,50 @@ static PyMappingMethods mappingproxy_as_mapping = {
10611061
0, /* mp_ass_subscript */
10621062
};
10631063

1064+
/* Use a copy of proxied mappings for operator dispatch, so reflected
1065+
methods cannot access the underlying mapping. */
1066+
static PyObject *
1067+
mappingproxy_copy_mapping(PyObject *mapping)
1068+
{
1069+
PyObject *copy_method;
1070+
int res = PyObject_GetOptionalAttr(mapping, &_Py_ID(copy), &copy_method);
1071+
if (res < 0) {
1072+
return NULL;
1073+
}
1074+
if (res == 0) {
1075+
Py_RETURN_NOTIMPLEMENTED;
1076+
}
1077+
1078+
PyObject *copy = PyObject_CallNoArgs(copy_method);
1079+
Py_DECREF(copy_method);
1080+
return copy;
1081+
}
1082+
1083+
static PyObject *
1084+
mappingproxy_as_operand(PyObject *operand)
1085+
{
1086+
if (PyObject_TypeCheck(operand, &PyDictProxy_Type)) {
1087+
return mappingproxy_copy_mapping(((mappingproxyobject *)operand)->mapping);
1088+
}
1089+
return Py_NewRef(operand);
1090+
}
1091+
10641092
static PyObject *
10651093
mappingproxy_or(PyObject *left, PyObject *right)
10661094
{
1067-
if (PyObject_TypeCheck(left, &PyDictProxy_Type)) {
1068-
left = ((mappingproxyobject*)left)->mapping;
1095+
left = mappingproxy_as_operand(left);
1096+
if (left == NULL || left == Py_NotImplemented) {
1097+
return left;
10691098
}
1070-
if (PyObject_TypeCheck(right, &PyDictProxy_Type)) {
1071-
right = ((mappingproxyobject*)right)->mapping;
1099+
right = mappingproxy_as_operand(right);
1100+
if (right == NULL || right == Py_NotImplemented) {
1101+
Py_DECREF(left);
1102+
return right;
10721103
}
1073-
return PyNumber_Or(left, right);
1104+
PyObject *result = PyNumber_Or(left, right);
1105+
Py_DECREF(left);
1106+
Py_DECREF(right);
1107+
return result;
10741108
}
10751109

10761110
static PyObject *
@@ -1234,7 +1268,13 @@ mappingproxy_richcompare(PyObject *self, PyObject *w, int op)
12341268
{
12351269
mappingproxyobject *v = (mappingproxyobject *)self;
12361270
if (op == Py_EQ || op == Py_NE) {
1237-
return PyObject_RichCompare(v->mapping, w, op);
1271+
PyObject *mapping = mappingproxy_copy_mapping(v->mapping);
1272+
if (mapping == NULL || mapping == Py_NotImplemented) {
1273+
return mapping;
1274+
}
1275+
PyObject *result = PyObject_RichCompare(mapping, w, op);
1276+
Py_DECREF(mapping);
1277+
return result;
12381278
}
12391279
Py_RETURN_NOTIMPLEMENTED;
12401280
}

0 commit comments

Comments
 (0)