Skip to content

Commit edabbff

Browse files
vstinnerJoekrry
andauthored
[3.13] gh-157335: Fix out-of-bounds write in mmap.mmap.__setitem__ (#157438) (#158028)
gh-157335: Fix out-of-bounds write in mmap.mmap.__setitem__ (#157438) Fix out-of-bounds write in mmap.mmap.__setitem__() that could occur when converting the index or the assigned value (via __index__() for a single item, or via the buffer protocol for a slice) resized or closed the mmap object during the assignment. (cherry picked from commit 09bf4c5) Co-authored-by: Joseph Kerry <joerkerry@gmail.com>
1 parent 50e0bc0 commit edabbff

3 files changed

Lines changed: 73 additions & 15 deletions

File tree

‎Lib/test/test_mmap.py‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def test_basic(self):
7171

7272
# Shouldn't crash on boundary (Issue #5292)
7373
self.assertRaises(IndexError, m.__getitem__, len(m))
74-
self.assertRaises(IndexError, m.__setitem__, len(m), b'\0')
74+
self.assertRaises(IndexError, m.__setitem__, len(m), 0)
7575

7676
# Modify the file's content
7777
m[0] = b'3'[0]
@@ -964,6 +964,55 @@ def test_resize_down_anonymous_mapping(self):
964964
with self.assertRaises(ValueError):
965965
m.resize(start_size)
966966

967+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
968+
def test_setitem_resize_reentrancy(self):
969+
"""Resizing the mmap from inside __index__ while assigning to a
970+
single item must not access memory past the new bounds (gh-157335).
971+
"""
972+
size = 2 * PAGESIZE
973+
new_size = PAGESIZE
974+
975+
class ResizeOnIndex:
976+
def __init__(self, m):
977+
self.m = m
978+
def __index__(self):
979+
self.m.resize(new_size)
980+
return 0
981+
982+
with mmap.mmap(-1, size) as m:
983+
try:
984+
with self.assertRaises(IndexError):
985+
m[size - 1] = ResizeOnIndex(m)
986+
except SystemError as exc:
987+
self.skipTest(f"resize() is not available: {exc!r}")
988+
self.assertEqual(len(m), new_size)
989+
990+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
991+
def test_setitem_slice_resize_reentrancy(self):
992+
"""Resizing the mmap from inside a value's buffer-protocol
993+
callback while assigning to a slice must not access memory past
994+
the new bounds (gh-157335).
995+
"""
996+
size = 2 * PAGESIZE
997+
new_size = PAGESIZE
998+
999+
class ResizeOnBuffer:
1000+
def __init__(self, m, data):
1001+
self.m = m
1002+
self.data = data
1003+
def __buffer__(self, flags):
1004+
self.m.resize(new_size)
1005+
return memoryview(self.data)
1006+
1007+
with mmap.mmap(-1, size) as m:
1008+
value = ResizeOnBuffer(m, bytes(size))
1009+
try:
1010+
with self.assertRaises(IndexError):
1011+
m[0:size] = value
1012+
except SystemError as exc:
1013+
self.skipTest(f"resize() is not available: {exc!r}")
1014+
self.assertEqual(len(m), new_size)
1015+
9671016
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
9681017
def test_resize_fails_if_mapping_held_elsewhere(self):
9691018
"""If more than one mapping is held against a named file on Windows, neither
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix out-of-bounds write in ``mmap.mmap.__setitem__`` that could occur
2+
when converting the index or the assigned value (via :meth:`~object.__index__`
3+
for a single item, or via the buffer protocol for a slice) resized or closed the mmap
4+
object during the assignment.

‎Modules/mmapmodule.c‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,24 +1347,14 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v)
13471347
static int
13481348
mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
13491349
{
1350-
CHECK_VALID(-1);
1351-
13521350
if (!is_writable(self))
13531351
return -1;
13541352

13551353
if (PyIndex_Check(item)) {
13561354
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
1357-
Py_ssize_t v;
1358-
13591355
if (i == -1 && PyErr_Occurred())
13601356
return -1;
1361-
if (i < 0)
1362-
i += self->size;
1363-
if (i < 0 || i >= self->size) {
1364-
PyErr_SetString(PyExc_IndexError,
1365-
"mmap index out of range");
1366-
return -1;
1367-
}
1357+
13681358
if (value == NULL) {
13691359
PyErr_SetString(PyExc_TypeError,
13701360
"mmap doesn't support item deletion");
@@ -1375,7 +1365,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
13751365
"mmap item value must be an int");
13761366
return -1;
13771367
}
1378-
v = PyNumber_AsSsize_t(value, PyExc_TypeError);
1368+
Py_ssize_t v = PyNumber_AsSsize_t(value, PyExc_TypeError);
13791369
if (v == -1 && PyErr_Occurred())
13801370
return -1;
13811371
if (v < 0 || v > 255) {
@@ -1384,7 +1374,18 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
13841374
"in range(0, 256)");
13851375
return -1;
13861376
}
1377+
1378+
/* Converting item or value above may have run arbitrary code
1379+
* (e.g. __index__) that resized or closed the mmap, so bounds
1380+
* are only checked now, against the current size. */
13871381
CHECK_VALID(-1);
1382+
if (i < 0)
1383+
i += self->size;
1384+
if (i < 0 || i >= self->size) {
1385+
PyErr_SetString(PyExc_IndexError,
1386+
"mmap index out of range");
1387+
return -1;
1388+
}
13881389

13891390
char v_char = (char) v;
13901391
if (safe_byte_copy(self->data + i, &v_char) < 0) {
@@ -1399,22 +1400,26 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
13991400
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
14001401
return -1;
14011402
}
1402-
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
14031403
if (value == NULL) {
14041404
PyErr_SetString(PyExc_TypeError,
14051405
"mmap object doesn't support slice deletion");
14061406
return -1;
14071407
}
14081408
if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0)
14091409
return -1;
1410+
1411+
/* Acquiring the buffer above may have run arbitrary code (e.g. a
1412+
* __buffer__ method) that resized or closed this mmap, so the slice bounds
1413+
* are only computed now, against the current size. */
1414+
CHECK_VALID_OR_RELEASE(-1, vbuf);
1415+
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
14101416
if (vbuf.len != slicelen) {
14111417
PyErr_SetString(PyExc_IndexError,
14121418
"mmap slice assignment is wrong size");
14131419
PyBuffer_Release(&vbuf);
14141420
return -1;
14151421
}
14161422

1417-
CHECK_VALID_OR_RELEASE(-1, vbuf);
14181423
int result = 0;
14191424
if (slicelen == 0) {
14201425
}

0 commit comments

Comments
 (0)