Skip to content

Commit 09bf4c5

Browse files
Joekrryvstinner
andauthored
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. Co-authored-by: Victor Stinner <vstinner@python.org>
1 parent 4bc392c commit 09bf4c5

3 files changed

Lines changed: 67 additions & 14 deletions

File tree

‎Lib/test/test_mmap.py‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def test_basic(self):
7575

7676
# Shouldn't crash on boundary (Issue #5292)
7777
self.assertRaises(IndexError, m.__getitem__, len(m))
78-
self.assertRaises(IndexError, m.__setitem__, len(m), b'\0')
78+
self.assertRaises(IndexError, m.__setitem__, len(m), 0)
7979

8080
# Modify the file's content
8181
m[0] = b'3'[0]
@@ -953,6 +953,49 @@ def test_resize_down_anonymous_mapping(self):
953953
with self.assertRaises(ValueError):
954954
m.resize(start_size)
955955

956+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
957+
def test_setitem_resize_reentrancy(self):
958+
"""Resizing the mmap from inside __index__ while assigning to a
959+
single item must not access memory past the new bounds (gh-157335).
960+
"""
961+
size = 2 * PAGESIZE
962+
new_size = PAGESIZE
963+
964+
class ResizeOnIndex:
965+
def __init__(self, m):
966+
self.m = m
967+
def __index__(self):
968+
self.m.resize(new_size)
969+
return 0
970+
971+
with mmap.mmap(-1, size) as m:
972+
with self.assertRaises(IndexError):
973+
m[size - 1] = ResizeOnIndex(m)
974+
self.assertEqual(len(m), new_size)
975+
976+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
977+
def test_setitem_slice_resize_reentrancy(self):
978+
"""Resizing the mmap from inside a value's buffer-protocol
979+
callback while assigning to a slice must not access memory past
980+
the new bounds (gh-157335).
981+
"""
982+
size = 2 * PAGESIZE
983+
new_size = PAGESIZE
984+
985+
class ResizeOnBuffer:
986+
def __init__(self, m, data):
987+
self.m = m
988+
self.data = data
989+
def __buffer__(self, flags):
990+
self.m.resize(new_size)
991+
return memoryview(self.data)
992+
993+
with mmap.mmap(-1, size) as m:
994+
value = ResizeOnBuffer(m, bytes(size))
995+
with self.assertRaises(IndexError):
996+
m[0:size] = value
997+
self.assertEqual(len(m), new_size)
998+
956999
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
9571000
def test_resize_fails_if_mapping_held_elsewhere(self):
9581001
"""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 & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,24 +1649,15 @@ static int
16491649
mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16501650
{
16511651
mmap_object *self = mmap_object_CAST(op);
1652-
CHECK_VALID(-1);
16531652

16541653
if (!is_writable(self))
16551654
return -1;
16561655

16571656
if (PyIndex_Check(item)) {
16581657
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
1659-
Py_ssize_t v;
1660-
16611658
if (i == -1 && PyErr_Occurred())
16621659
return -1;
1663-
if (i < 0)
1664-
i += self->size;
1665-
if (i < 0 || i >= self->size) {
1666-
PyErr_SetString(PyExc_IndexError,
1667-
"mmap index out of range");
1668-
return -1;
1669-
}
1660+
16701661
if (value == NULL) {
16711662
PyErr_SetString(PyExc_TypeError,
16721663
"mmap doesn't support item deletion");
@@ -1677,7 +1668,7 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16771668
"mmap item value must be an int");
16781669
return -1;
16791670
}
1680-
v = PyNumber_AsSsize_t(value, PyExc_TypeError);
1671+
Py_ssize_t v = PyNumber_AsSsize_t(value, PyExc_TypeError);
16811672
if (v == -1 && PyErr_Occurred())
16821673
return -1;
16831674
if (v < 0 || v > 255) {
@@ -1686,7 +1677,18 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16861677
"in range(0, 256)");
16871678
return -1;
16881679
}
1680+
1681+
/* Converting item or value above may have run arbitrary code
1682+
* (e.g. __index__) that resized or closed the mmap, so bounds
1683+
* are only checked now, against the current size. */
16891684
CHECK_VALID(-1);
1685+
if (i < 0)
1686+
i += self->size;
1687+
if (i < 0 || i >= self->size) {
1688+
PyErr_SetString(PyExc_IndexError,
1689+
"mmap index out of range");
1690+
return -1;
1691+
}
16901692

16911693
char v_char = (char) v;
16921694
if (safe_byte_copy(self->data + i, &v_char) < 0) {
@@ -1701,22 +1703,26 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
17011703
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
17021704
return -1;
17031705
}
1704-
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
17051706
if (value == NULL) {
17061707
PyErr_SetString(PyExc_TypeError,
17071708
"mmap object doesn't support slice deletion");
17081709
return -1;
17091710
}
17101711
if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0)
17111712
return -1;
1713+
1714+
/* Acquiring the buffer above may have run arbitrary code (e.g. a
1715+
* __buffer__ method) that resized or closed this mmap, so the slice bounds
1716+
* are only computed now, against the current size. */
1717+
CHECK_VALID_OR_RELEASE(-1, vbuf);
1718+
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
17121719
if (vbuf.len != slicelen) {
17131720
PyErr_SetString(PyExc_IndexError,
17141721
"mmap slice assignment is wrong size");
17151722
PyBuffer_Release(&vbuf);
17161723
return -1;
17171724
}
17181725

1719-
CHECK_VALID_OR_RELEASE(-1, vbuf);
17201726
int result = 0;
17211727
if (slicelen == 0) {
17221728
}

0 commit comments

Comments
 (0)