Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-92888: Fix memoryview bad __index__ use after free #92946

Merged
merged 12 commits into from
Jun 17, 2022
22 changes: 22 additions & 0 deletions Lib/test/test_memoryview.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,28 @@ def test_pickle(self):
with self.assertRaises(TypeError):
pickle.dumps(m, proto)

def test_memoryview_bad_index_uaf(self):
# memoryview Use After Free (memory_ass_sub) see gh-92888
uaf_backing = bytearray(bytearray.__basicsize__)
uaf_view = memoryview(uaf_backing).cast('n') # ssize_t format
memory_backing = None

class weird_index:
def __index__(self):
nonlocal memory_backing
uaf_view.release() # release memoryview (UAF)
# free `uaf_backing` memory and allocate a new bytearray into it
memory_backing = uaf_backing.clear() or bytearray()
return 2 # `ob_size` idx

# by the time this line finishes executing, it writes the max ptr size
# into the `ob_size` slot of `memory_backing`
with self.assertRaisesRegex(ValueError, "operation forbidden on released memoryview object"):
uaf_view[weird_index()] = (2 ** (tuple.__itemsize__ * 8) - 1) // 2
memory = memoryview(memory_backing)
with self.assertRaisesRegex(IndexError, "index out of bounds"):
memory[id(250) + int.__basicsize__] = 100
self.assertEqual(250, eval("250"))

if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``memoryview`` use after free when subscripting an object with bad ``__index__``. This occurs when the backing buffer is released inside ``__index__``.
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 14 additions & 4 deletions Objects/memoryobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ unpack_single(const char *ptr, const char *fmt)
/* Pack a single item. 'fmt' can be any native format character in
struct module syntax. */
static int
pack_single(char *ptr, PyObject *item, const char *fmt)
pack_single(PyMemoryViewObject *self, char *ptr, PyObject *item, const char *fmt)
{
unsigned long long llu;
unsigned long lu;
Expand All @@ -1784,6 +1784,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
ld = pylong_as_ld(item);
if (ld == -1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
switch (fmt[0]) {
case 'b':
if (ld < SCHAR_MIN || ld > SCHAR_MAX) goto err_range;
Expand All @@ -1804,6 +1805,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
lu = pylong_as_lu(item);
if (lu == (unsigned long)-1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
switch (fmt[0]) {
case 'B':
if (lu > UCHAR_MAX) goto err_range;
Expand All @@ -1824,12 +1826,14 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
lld = pylong_as_lld(item);
if (lld == -1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, lld, long long);
break;
case 'Q':
llu = pylong_as_llu(item);
if (llu == (unsigned long long)-1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, llu, unsigned long long);
break;

Expand All @@ -1838,12 +1842,14 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
zd = pylong_as_zd(item);
if (zd == -1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, zd, Py_ssize_t);
break;
case 'N':
zu = pylong_as_zu(item);
if (zu == (size_t)-1 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, zu, size_t);
break;

Expand All @@ -1852,6 +1858,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
d = PyFloat_AsDouble(item);
if (d == -1.0 && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
if (fmt[0] == 'f') {
PACK_SINGLE(ptr, d, float);
}
Expand All @@ -1865,6 +1872,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
ld = PyObject_IsTrue(item);
if (ld < 0)
return -1; /* preserve original error */
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, ld, _Bool);
break;

Expand All @@ -1882,6 +1890,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
p = PyLong_AsVoidPtr(item);
if (p == NULL && PyErr_Occurred())
goto err_occurred;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
PACK_SINGLE(ptr, p, void *);
break;

Expand Down Expand Up @@ -2538,7 +2547,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
if (key == Py_Ellipsis ||
(PyTuple_Check(key) && PyTuple_GET_SIZE(key)==0)) {
ptr = (char *)view->buf;
return pack_single(ptr, value, fmt);
return pack_single(self, ptr, value, fmt);
}
else {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -2557,10 +2566,11 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
index = PyNumber_AsSsize_t(key, PyExc_IndexError);
if (index == -1 && PyErr_Occurred())
return -1;
CHECK_RELEASED_INT(self); /* See gh-92888 for why we need this here */
ptr = ptr_from_index(view, index);
if (ptr == NULL)
return -1;
return pack_single(ptr, value, fmt);
return pack_single(self, ptr, value, fmt);
}
/* one-dimensional: fast path */
if (PySlice_Check(key) && view->ndim == 1) {
Expand Down Expand Up @@ -2599,7 +2609,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
ptr = ptr_from_tuple(view, key);
if (ptr == NULL)
return -1;
return pack_single(ptr, value, fmt);
return pack_single(self, ptr, value, fmt);
}
if (PySlice_Check(key) || is_multislice(key)) {
/* Call memory_subscript() to produce a sliced lvalue, then copy
Expand Down