[Python-checkins] gh-92888: Fix memoryview bad `__index__` use after free (GH-92946) (GH-93950)
Fidget-Spinner
webhook-mailer at python.org
Thu Jun 23 06:10:27 EDT 2022
https://github.com/python/cpython/commit/73b1d494a7cb46fc2cd45b0616647e4cfb786f3d
commit: 73b1d494a7cb46fc2cd45b0616647e4cfb786f3d
branch: 3.10
author: Ken Jin <28750310+Fidget-Spinner at users.noreply.github.com>
committer: Fidget-Spinner <28750310+Fidget-Spinner at users.noreply.github.com>
date: 2022-06-23T18:10:14+08:00
summary:
gh-92888: Fix memoryview bad `__index__` use after free (GH-92946) (GH-93950)
(cherry picked from commit 11190c4ad0d3722b8d263758ac802985131a5462)
Co-authored-by: chilaxan <35645806+chilaxan at users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <3659035+serhiy-storchaka at users.noreply.github.com>
files:
A Misc/NEWS.d/next/Security/2022-05-19-08-53-07.gh-issue-92888.TLtR9W.rst
M Lib/test/test_memoryview.py
M Objects/memoryobject.c
diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py
index d7e3f0c0effa6..9d1e1f3063ca5 100644
--- a/Lib/test/test_memoryview.py
+++ b/Lib/test/test_memoryview.py
@@ -545,6 +545,107 @@ def test_pickle(self):
with self.assertRaises(TypeError):
pickle.dumps(m, proto)
+ def test_use_released_memory(self):
+ # gh-92888: Previously it was possible to use a memoryview even after
+ # backing buffer is freed in certain cases. This tests that those
+ # cases raise an exception.
+ size = 128
+ def release():
+ m.release()
+ nonlocal ba
+ ba = bytearray(size)
+ class MyIndex:
+ def __index__(self):
+ release()
+ return 4
+ class MyFloat:
+ def __float__(self):
+ release()
+ return 4.25
+ class MyBool:
+ def __bool__(self):
+ release()
+ return True
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ with self.assertRaises(ValueError):
+ m[MyIndex()]
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ self.assertEqual(list(m[:MyIndex()]), [255] * 4)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ self.assertEqual(list(m[MyIndex():8]), [255] * 4)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast('B', (64, 2))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[MyIndex(), 0]
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast('B', (2, 64))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0, MyIndex()]
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[MyIndex()] = 42
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[:MyIndex()] = b'spam'
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[MyIndex():8] = b'spam'
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast('B', (64, 2))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[MyIndex(), 0] = 42
+ self.assertEqual(ba[8:16], b'\0'*8)
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast('B', (2, 64))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0, MyIndex()] = 42
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size))
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0] = MyIndex()
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ for fmt in 'bhilqnBHILQN':
+ with self.subTest(fmt=fmt):
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast(fmt)
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0] = MyIndex()
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ for fmt in 'fd':
+ with self.subTest(fmt=fmt):
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast(fmt)
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0] = MyFloat()
+ self.assertEqual(ba[:8], b'\0'*8)
+
+ ba = None
+ m = memoryview(bytearray(b'\xff'*size)).cast('?')
+ with self.assertRaisesRegex(ValueError, "operation forbidden"):
+ m[0] = MyBool()
+ self.assertEqual(ba[:8], b'\0'*8)
if __name__ == "__main__":
unittest.main()
diff --git a/Misc/NEWS.d/next/Security/2022-05-19-08-53-07.gh-issue-92888.TLtR9W.rst b/Misc/NEWS.d/next/Security/2022-05-19-08-53-07.gh-issue-92888.TLtR9W.rst
new file mode 100644
index 0000000000000..4841b8a90a087
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2022-05-19-08-53-07.gh-issue-92888.TLtR9W.rst
@@ -0,0 +1,2 @@
+Fix ``memoryview`` use after free when accessing the backing buffer in certain cases.
+
diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c
index 913d358062219..f494073ba6bd2 100644
--- a/Objects/memoryobject.c
+++ b/Objects/memoryobject.c
@@ -201,6 +201,11 @@ PyTypeObject _PyManagedBuffer_Type = {
return -1; \
}
+/* See gh-92888. These macros signal that we need to check the memoryview
+ again due to possible read after frees. */
+#define CHECK_RELEASED_AGAIN(mv) CHECK_RELEASED(mv)
+#define CHECK_RELEASED_INT_AGAIN(mv) CHECK_RELEASED_INT(mv)
+
#define CHECK_LIST_OR_TUPLE(v) \
if (!PyList_Check(v) && !PyTuple_Check(v)) { \
PyErr_SetString(PyExc_TypeError, \
@@ -389,8 +394,9 @@ copy_rec(const Py_ssize_t *shape, Py_ssize_t ndim, Py_ssize_t itemsize,
/* Faster copying of one-dimensional arrays. */
static int
-copy_single(Py_buffer *dest, Py_buffer *src)
+copy_single(PyMemoryViewObject *self, const Py_buffer *dest, const Py_buffer *src)
{
+ CHECK_RELEASED_INT_AGAIN(self);
char *mem = NULL;
assert(dest->ndim == 1);
@@ -1685,7 +1691,7 @@ pylong_as_zu(PyObject *item)
module syntax. This function is very sensitive to small changes. With this
layout gcc automatically generates a fast jump table. */
static inline PyObject *
-unpack_single(const char *ptr, const char *fmt)
+unpack_single(PyMemoryViewObject *self, const char *ptr, const char *fmt)
{
unsigned long long llu;
unsigned long lu;
@@ -1697,6 +1703,8 @@ unpack_single(const char *ptr, const char *fmt)
unsigned char uc;
void *p;
+ CHECK_RELEASED_AGAIN(self);
+
switch (fmt[0]) {
/* signed integers and fast path for 'B' */
@@ -1775,7 +1783,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;
@@ -1792,6 +1800,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_AGAIN(self);
switch (fmt[0]) {
case 'b':
if (ld < SCHAR_MIN || ld > SCHAR_MAX) goto err_range;
@@ -1812,6 +1821,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_AGAIN(self);
switch (fmt[0]) {
case 'B':
if (lu > UCHAR_MAX) goto err_range;
@@ -1832,12 +1842,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_AGAIN(self);
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_AGAIN(self);
PACK_SINGLE(ptr, llu, unsigned long long);
break;
@@ -1846,12 +1858,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_AGAIN(self);
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_AGAIN(self);
PACK_SINGLE(ptr, zu, size_t);
break;
@@ -1860,6 +1874,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_AGAIN(self);
if (fmt[0] == 'f') {
PACK_SINGLE(ptr, d, float);
}
@@ -1873,6 +1888,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_AGAIN(self);
PACK_SINGLE(ptr, ld, _Bool);
break;
@@ -1890,6 +1906,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_AGAIN(self);
PACK_SINGLE(ptr, p, void *);
break;
@@ -2056,7 +2073,7 @@ adjust_fmt(const Py_buffer *view)
/* Base case for multi-dimensional unpacking. Assumption: ndim == 1. */
static PyObject *
-tolist_base(const char *ptr, const Py_ssize_t *shape,
+tolist_base(PyMemoryViewObject *self, const char *ptr, const Py_ssize_t *shape,
const Py_ssize_t *strides, const Py_ssize_t *suboffsets,
const char *fmt)
{
@@ -2069,7 +2086,7 @@ tolist_base(const char *ptr, const Py_ssize_t *shape,
for (i = 0; i < shape[0]; ptr+=strides[0], i++) {
const char *xptr = ADJUST_PTR(ptr, suboffsets, 0);
- item = unpack_single(xptr, fmt);
+ item = unpack_single(self, xptr, fmt);
if (item == NULL) {
Py_DECREF(lst);
return NULL;
@@ -2083,7 +2100,7 @@ tolist_base(const char *ptr, const Py_ssize_t *shape,
/* Unpack a multi-dimensional array into a nested list.
Assumption: ndim >= 1. */
static PyObject *
-tolist_rec(const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
+tolist_rec(PyMemoryViewObject *self, const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
const Py_ssize_t *strides, const Py_ssize_t *suboffsets,
const char *fmt)
{
@@ -2095,7 +2112,7 @@ tolist_rec(const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
assert(strides != NULL);
if (ndim == 1)
- return tolist_base(ptr, shape, strides, suboffsets, fmt);
+ return tolist_base(self, ptr, shape, strides, suboffsets, fmt);
lst = PyList_New(shape[0]);
if (lst == NULL)
@@ -2103,7 +2120,7 @@ tolist_rec(const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
for (i = 0; i < shape[0]; ptr+=strides[0], i++) {
const char *xptr = ADJUST_PTR(ptr, suboffsets, 0);
- item = tolist_rec(xptr, ndim-1, shape+1,
+ item = tolist_rec(self, xptr, ndim-1, shape+1,
strides+1, suboffsets ? suboffsets+1 : NULL,
fmt);
if (item == NULL) {
@@ -2137,15 +2154,15 @@ memoryview_tolist_impl(PyMemoryViewObject *self)
if (fmt == NULL)
return NULL;
if (view->ndim == 0) {
- return unpack_single(view->buf, fmt);
+ return unpack_single(self, view->buf, fmt);
}
else if (view->ndim == 1) {
- return tolist_base(view->buf, view->shape,
+ return tolist_base(self, view->buf, view->shape,
view->strides, view->suboffsets,
fmt);
}
else {
- return tolist_rec(view->buf, view->ndim, view->shape,
+ return tolist_rec(self, view->buf, view->ndim, view->shape,
view->strides, view->suboffsets,
fmt);
}
@@ -2353,7 +2370,7 @@ memory_item(PyMemoryViewObject *self, Py_ssize_t index)
char *ptr = ptr_from_index(view, index);
if (ptr == NULL)
return NULL;
- return unpack_single(ptr, fmt);
+ return unpack_single(self, ptr, fmt);
}
PyErr_SetString(PyExc_NotImplementedError,
@@ -2384,7 +2401,7 @@ memory_item_multi(PyMemoryViewObject *self, PyObject *tup)
ptr = ptr_from_tuple(view, tup);
if (ptr == NULL)
return NULL;
- return unpack_single(ptr, fmt);
+ return unpack_single(self, ptr, fmt);
}
static inline int
@@ -2471,7 +2488,7 @@ memory_subscript(PyMemoryViewObject *self, PyObject *key)
const char *fmt = adjust_fmt(view);
if (fmt == NULL)
return NULL;
- return unpack_single(view->buf, fmt);
+ return unpack_single(self, view->buf, fmt);
}
else if (key == Py_Ellipsis) {
Py_INCREF(self);
@@ -2546,7 +2563,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,
@@ -2568,7 +2585,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
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) {
@@ -2591,7 +2608,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
goto end_block;
dest.len = dest.shape[0] * dest.itemsize;
- ret = copy_single(&dest, &src);
+ ret = copy_single(self, &dest, &src);
end_block:
PyBuffer_Release(&src);
@@ -2607,7 +2624,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
@@ -3208,7 +3225,7 @@ memoryiter_next(memoryiterobject *it)
if (ptr == NULL) {
return NULL;
}
- return unpack_single(ptr, it->it_fmt);
+ return unpack_single(seq, ptr, it->it_fmt);
}
it->it_seq = NULL;
More information about the Python-checkins
mailing list