Skip to content

Commit bfd7049

Browse files
Fidget-Spinnerchilaxanserhiy-storchaka
committed
pythongh-92888: Fix memoryview bad __index__ use after free (pythonGH-92946)
Co-authored-by: chilaxan <35645806+chilaxan@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <3659035+serhiy-storchaka@users.noreply.github.com>
1 parent 5e30ba1 commit bfd7049

File tree

3 files changed

+139
-19
lines changed

3 files changed

+139
-19
lines changed

‎Lib/test/test_memoryview.py‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,107 @@ def test_pickle(self):
545545
with self.assertRaises(TypeError):
546546
pickle.dumps(m, proto)
547547

548+
def test_use_released_memory(self):
549+
# gh-92888: Previously it was possible to use a memoryview even after
550+
# backing buffer is freed in certain cases. This tests that those
551+
# cases raise an exception.
552+
size = 128
553+
def release():
554+
m.release()
555+
nonlocal ba
556+
ba = bytearray(size)
557+
class MyIndex:
558+
def __index__(self):
559+
release()
560+
return 4
561+
class MyFloat:
562+
def __float__(self):
563+
release()
564+
return 4.25
565+
class MyBool:
566+
def __bool__(self):
567+
release()
568+
return True
569+
570+
ba = None
571+
m = memoryview(bytearray(b'\xff'*size))
572+
with self.assertRaises(ValueError):
573+
m[MyIndex()]
574+
575+
ba = None
576+
m = memoryview(bytearray(b'\xff'*size))
577+
self.assertEqual(list(m[:MyIndex()]), [255] * 4)
578+
579+
ba = None
580+
m = memoryview(bytearray(b'\xff'*size))
581+
self.assertEqual(list(m[MyIndex():8]), [255] * 4)
582+
583+
ba = None
584+
m = memoryview(bytearray(b'\xff'*size)).cast('B', (64, 2))
585+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
586+
m[MyIndex(), 0]
587+
588+
ba = None
589+
m = memoryview(bytearray(b'\xff'*size)).cast('B', (2, 64))
590+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
591+
m[0, MyIndex()]
592+
593+
ba = None
594+
m = memoryview(bytearray(b'\xff'*size))
595+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
596+
m[MyIndex()] = 42
597+
self.assertEqual(ba[:8], b'\0'*8)
598+
599+
ba = None
600+
m = memoryview(bytearray(b'\xff'*size))
601+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
602+
m[:MyIndex()] = b'spam'
603+
self.assertEqual(ba[:8], b'\0'*8)
604+
605+
ba = None
606+
m = memoryview(bytearray(b'\xff'*size))
607+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
608+
m[MyIndex():8] = b'spam'
609+
self.assertEqual(ba[:8], b'\0'*8)
610+
611+
ba = None
612+
m = memoryview(bytearray(b'\xff'*size)).cast('B', (64, 2))
613+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
614+
m[MyIndex(), 0] = 42
615+
self.assertEqual(ba[8:16], b'\0'*8)
616+
ba = None
617+
m = memoryview(bytearray(b'\xff'*size)).cast('B', (2, 64))
618+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
619+
m[0, MyIndex()] = 42
620+
self.assertEqual(ba[:8], b'\0'*8)
621+
622+
ba = None
623+
m = memoryview(bytearray(b'\xff'*size))
624+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
625+
m[0] = MyIndex()
626+
self.assertEqual(ba[:8], b'\0'*8)
627+
628+
for fmt in 'bhilqnBHILQN':
629+
with self.subTest(fmt=fmt):
630+
ba = None
631+
m = memoryview(bytearray(b'\xff'*size)).cast(fmt)
632+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
633+
m[0] = MyIndex()
634+
self.assertEqual(ba[:8], b'\0'*8)
635+
636+
for fmt in 'fd':
637+
with self.subTest(fmt=fmt):
638+
ba = None
639+
m = memoryview(bytearray(b'\xff'*size)).cast(fmt)
640+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
641+
m[0] = MyFloat()
642+
self.assertEqual(ba[:8], b'\0'*8)
643+
644+
ba = None
645+
m = memoryview(bytearray(b'\xff'*size)).cast('?')
646+
with self.assertRaisesRegex(ValueError, "operation forbidden"):
647+
m[0] = MyBool()
648+
self.assertEqual(ba[:8], b'\0'*8)
548649

549650
if __name__ == "__main__":
550651
unittest.main()

‎Misc/NEWS.d/next/Security/2022-05-19-08-53-07.gh-issue-92888.TLtR9W.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix ``memoryview`` use after free when accessing the backing buffer in certain cases.
2+

‎Objects/memoryobject.c‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ PyTypeObject _PyManagedBuffer_Type = {
201201
return -1; \
202202
}
203203

204+
/* See gh-92888. These macros signal that we need to check the memoryview
205+
again due to possible read after frees. */
206+
#define CHECK_RELEASED_AGAIN(mv) CHECK_RELEASED(mv)
207+
#define CHECK_RELEASED_INT_AGAIN(mv) CHECK_RELEASED_INT(mv)
208+
204209
#define CHECK_LIST_OR_TUPLE(v) \
205210
if (!PyList_Check(v) && !PyTuple_Check(v)) { \
206211
PyErr_SetString(PyExc_TypeError, \
@@ -389,8 +394,9 @@ copy_rec(const Py_ssize_t *shape, Py_ssize_t ndim, Py_ssize_t itemsize,
389394

390395
/* Faster copying of one-dimensional arrays. */
391396
static int
392-
copy_single(Py_buffer *dest, Py_buffer *src)
397+
copy_single(PyMemoryViewObject *self, const Py_buffer *dest, const Py_buffer *src)
393398
{
399+
CHECK_RELEASED_INT_AGAIN(self);
394400
char *mem = NULL;
395401

396402
assert(dest->ndim == 1);
@@ -1685,7 +1691,7 @@ pylong_as_zu(PyObject *item)
16851691
module syntax. This function is very sensitive to small changes. With this
16861692
layout gcc automatically generates a fast jump table. */
16871693
static inline PyObject *
1688-
unpack_single(const char *ptr, const char *fmt)
1694+
unpack_single(PyMemoryViewObject *self, const char *ptr, const char *fmt)
16891695
{
16901696
unsigned long long llu;
16911697
unsigned long lu;
@@ -1697,6 +1703,8 @@ unpack_single(const char *ptr, const char *fmt)
16971703
unsigned char uc;
16981704
void *p;
16991705

1706+
CHECK_RELEASED_AGAIN(self);
1707+
17001708
switch (fmt[0]) {
17011709

17021710
/* signed integers and fast path for 'B' */
@@ -1775,7 +1783,7 @@ unpack_single(const char *ptr, const char *fmt)
17751783
/* Pack a single item. 'fmt' can be any native format character in
17761784
struct module syntax. */
17771785
static int
1778-
pack_single(char *ptr, PyObject *item, const char *fmt)
1786+
pack_single(PyMemoryViewObject *self, char *ptr, PyObject *item, const char *fmt)
17791787
{
17801788
unsigned long long llu;
17811789
unsigned long lu;
@@ -1792,6 +1800,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
17921800
ld = pylong_as_ld(item);
17931801
if (ld == -1 && PyErr_Occurred())
17941802
goto err_occurred;
1803+
CHECK_RELEASED_INT_AGAIN(self);
17951804
switch (fmt[0]) {
17961805
case 'b':
17971806
if (ld < SCHAR_MIN || ld > SCHAR_MAX) goto err_range;
@@ -1812,6 +1821,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18121821
lu = pylong_as_lu(item);
18131822
if (lu == (unsigned long)-1 && PyErr_Occurred())
18141823
goto err_occurred;
1824+
CHECK_RELEASED_INT_AGAIN(self);
18151825
switch (fmt[0]) {
18161826
case 'B':
18171827
if (lu > UCHAR_MAX) goto err_range;
@@ -1832,12 +1842,14 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18321842
lld = pylong_as_lld(item);
18331843
if (lld == -1 && PyErr_Occurred())
18341844
goto err_occurred;
1845+
CHECK_RELEASED_INT_AGAIN(self);
18351846
PACK_SINGLE(ptr, lld, long long);
18361847
break;
18371848
case 'Q':
18381849
llu = pylong_as_llu(item);
18391850
if (llu == (unsigned long long)-1 && PyErr_Occurred())
18401851
goto err_occurred;
1852+
CHECK_RELEASED_INT_AGAIN(self);
18411853
PACK_SINGLE(ptr, llu, unsigned long long);
18421854
break;
18431855

@@ -1846,12 +1858,14 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18461858
zd = pylong_as_zd(item);
18471859
if (zd == -1 && PyErr_Occurred())
18481860
goto err_occurred;
1861+
CHECK_RELEASED_INT_AGAIN(self);
18491862
PACK_SINGLE(ptr, zd, Py_ssize_t);
18501863
break;
18511864
case 'N':
18521865
zu = pylong_as_zu(item);
18531866
if (zu == (size_t)-1 && PyErr_Occurred())
18541867
goto err_occurred;
1868+
CHECK_RELEASED_INT_AGAIN(self);
18551869
PACK_SINGLE(ptr, zu, size_t);
18561870
break;
18571871

@@ -1860,6 +1874,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18601874
d = PyFloat_AsDouble(item);
18611875
if (d == -1.0 && PyErr_Occurred())
18621876
goto err_occurred;
1877+
CHECK_RELEASED_INT_AGAIN(self);
18631878
if (fmt[0] == 'f') {
18641879
PACK_SINGLE(ptr, d, float);
18651880
}
@@ -1873,6 +1888,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18731888
ld = PyObject_IsTrue(item);
18741889
if (ld < 0)
18751890
return -1; /* preserve original error */
1891+
CHECK_RELEASED_INT_AGAIN(self);
18761892
PACK_SINGLE(ptr, ld, _Bool);
18771893
break;
18781894

@@ -1890,6 +1906,7 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
18901906
p = PyLong_AsVoidPtr(item);
18911907
if (p == NULL && PyErr_Occurred())
18921908
goto err_occurred;
1909+
CHECK_RELEASED_INT_AGAIN(self);
18931910
PACK_SINGLE(ptr, p, void *);
18941911
break;
18951912

@@ -2056,7 +2073,7 @@ adjust_fmt(const Py_buffer *view)
20562073

20572074
/* Base case for multi-dimensional unpacking. Assumption: ndim == 1. */
20582075
static PyObject *
2059-
tolist_base(const char *ptr, const Py_ssize_t *shape,
2076+
tolist_base(PyMemoryViewObject *self, const char *ptr, const Py_ssize_t *shape,
20602077
const Py_ssize_t *strides, const Py_ssize_t *suboffsets,
20612078
const char *fmt)
20622079
{
@@ -2069,7 +2086,7 @@ tolist_base(const char *ptr, const Py_ssize_t *shape,
20692086

20702087
for (i = 0; i < shape[0]; ptr+=strides[0], i++) {
20712088
const char *xptr = ADJUST_PTR(ptr, suboffsets, 0);
2072-
item = unpack_single(xptr, fmt);
2089+
item = unpack_single(self, xptr, fmt);
20732090
if (item == NULL) {
20742091
Py_DECREF(lst);
20752092
return NULL;
@@ -2083,7 +2100,7 @@ tolist_base(const char *ptr, const Py_ssize_t *shape,
20832100
/* Unpack a multi-dimensional array into a nested list.
20842101
Assumption: ndim >= 1. */
20852102
static PyObject *
2086-
tolist_rec(const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
2103+
tolist_rec(PyMemoryViewObject *self, const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
20872104
const Py_ssize_t *strides, const Py_ssize_t *suboffsets,
20882105
const char *fmt)
20892106
{
@@ -2095,15 +2112,15 @@ tolist_rec(const char *ptr, Py_ssize_t ndim, const Py_ssize_t *shape,
20952112
assert(strides != NULL);
20962113

20972114
if (ndim == 1)
2098-
return tolist_base(ptr, shape, strides, suboffsets, fmt);
2115+
return tolist_base(self, ptr, shape, strides, suboffsets, fmt);
20992116

21002117
lst = PyList_New(shape[0]);
21012118
if (lst == NULL)
21022119
return NULL;
21032120

21042121
for (i = 0; i < shape[0]; ptr+=strides[0], i++) {
21052122
const char *xptr = ADJUST_PTR(ptr, suboffsets, 0);
2106-
item = tolist_rec(xptr, ndim-1, shape+1,
2123+
item = tolist_rec(self, xptr, ndim-1, shape+1,
21072124
strides+1, suboffsets ? suboffsets+1 : NULL,
21082125
fmt);
21092126
if (item == NULL) {
@@ -2137,15 +2154,15 @@ memoryview_tolist_impl(PyMemoryViewObject *self)
21372154
if (fmt == NULL)
21382155
return NULL;
21392156
if (view->ndim == 0) {
2140-
return unpack_single(view->buf, fmt);
2157+
return unpack_single(self, view->buf, fmt);
21412158
}
21422159
else if (view->ndim == 1) {
2143-
return tolist_base(view->buf, view->shape,
2160+
return tolist_base(self, view->buf, view->shape,
21442161
view->strides, view->suboffsets,
21452162
fmt);
21462163
}
21472164
else {
2148-
return tolist_rec(view->buf, view->ndim, view->shape,
2165+
return tolist_rec(self, view->buf, view->ndim, view->shape,
21492166
view->strides, view->suboffsets,
21502167
fmt);
21512168
}
@@ -2353,7 +2370,7 @@ memory_item(PyMemoryViewObject *self, Py_ssize_t index)
23532370
char *ptr = ptr_from_index(view, index);
23542371
if (ptr == NULL)
23552372
return NULL;
2356-
return unpack_single(ptr, fmt);
2373+
return unpack_single(self, ptr, fmt);
23572374
}
23582375

23592376
PyErr_SetString(PyExc_NotImplementedError,
@@ -2384,7 +2401,7 @@ memory_item_multi(PyMemoryViewObject *self, PyObject *tup)
23842401
ptr = ptr_from_tuple(view, tup);
23852402
if (ptr == NULL)
23862403
return NULL;
2387-
return unpack_single(ptr, fmt);
2404+
return unpack_single(self, ptr, fmt);
23882405
}
23892406

23902407
static inline int
@@ -2471,7 +2488,7 @@ memory_subscript(PyMemoryViewObject *self, PyObject *key)
24712488
const char *fmt = adjust_fmt(view);
24722489
if (fmt == NULL)
24732490
return NULL;
2474-
return unpack_single(view->buf, fmt);
2491+
return unpack_single(self, view->buf, fmt);
24752492
}
24762493
else if (key == Py_Ellipsis) {
24772494
Py_INCREF(self);
@@ -2546,7 +2563,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
25462563
if (key == Py_Ellipsis ||
25472564
(PyTuple_Check(key) && PyTuple_GET_SIZE(key)==0)) {
25482565
ptr = (char *)view->buf;
2549-
return pack_single(ptr, value, fmt);
2566+
return pack_single(self, ptr, value, fmt);
25502567
}
25512568
else {
25522569
PyErr_SetString(PyExc_TypeError,
@@ -2568,7 +2585,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
25682585
ptr = ptr_from_index(view, index);
25692586
if (ptr == NULL)
25702587
return -1;
2571-
return pack_single(ptr, value, fmt);
2588+
return pack_single(self, ptr, value, fmt);
25722589
}
25732590
/* one-dimensional: fast path */
25742591
if (PySlice_Check(key) && view->ndim == 1) {
@@ -2591,7 +2608,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
25912608
goto end_block;
25922609
dest.len = dest.shape[0] * dest.itemsize;
25932610

2594-
ret = copy_single(&dest, &src);
2611+
ret = copy_single(self, &dest, &src);
25952612

25962613
end_block:
25972614
PyBuffer_Release(&src);
@@ -2607,7 +2624,7 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
26072624
ptr = ptr_from_tuple(view, key);
26082625
if (ptr == NULL)
26092626
return -1;
2610-
return pack_single(ptr, value, fmt);
2627+
return pack_single(self, ptr, value, fmt);
26112628
}
26122629
if (PySlice_Check(key) || is_multislice(key)) {
26132630
/* Call memory_subscript() to produce a sliced lvalue, then copy
@@ -3208,7 +3225,7 @@ memoryiter_next(memoryiterobject *it)
32083225
if (ptr == NULL) {
32093226
return NULL;
32103227
}
3211-
return unpack_single(ptr, it->it_fmt);
3228+
return unpack_single(seq, ptr, it->it_fmt);
32123229
}
32133230

32143231
it->it_seq = NULL;

0 commit comments

Comments
 (0)