Skip to content

Commit ce88c7e

Browse files
committed
Fix UAF in _collections Counter update fast path
1 parent b8d3fdd commit ce88c7e

File tree

2 files changed

+21
-0
lines changed

2 files changed

+21
-0
lines changed

Lib/test/test_collections.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,6 +2135,22 @@ def test_basics(self):
21352135
self.assertEqual(c.setdefault('e', 5), 5)
21362136
self.assertEqual(c['e'], 5)
21372137

2138+
def test_update_reentrant_add_clears_counter(self):
2139+
c = Counter()
2140+
key = object()
2141+
2142+
class Evil(int):
2143+
def __new__(cls):
2144+
return int.__new__(cls, 0)
2145+
2146+
def __add__(self, other):
2147+
c.clear()
2148+
return NotImplemented
2149+
2150+
c[key] = Evil()
2151+
c.update([key])
2152+
self.assertEqual(c[key], 1)
2153+
21382154
def test_init(self):
21392155
self.assertEqual(list(Counter(self=42).items()), [('self', 42)])
21402156
self.assertEqual(list(Counter(iterable=42).items()), [('iterable', 42)])

Modules/_collectionsmodule.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,7 +2577,12 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
25772577
if (_PyDict_SetItem_KnownHash(mapping, key, one, hash) < 0)
25782578
goto done;
25792579
} else {
2580+
/* oldval is a borrowed reference. Keep it alive across
2581+
PyNumber_Add(), which can execute arbitrary user code and
2582+
mutate (or even clear) the underlying dict. */
2583+
Py_INCREF(oldval);
25802584
newval = PyNumber_Add(oldval, one);
2585+
Py_DECREF(oldval);
25812586
if (newval == NULL)
25822587
goto done;
25832588
if (_PyDict_SetItem_KnownHash(mapping, key, newval, hash) < 0)

0 commit comments

Comments
 (0)