Skip to content

Commit 960f69c

Browse files
miss-islingtonJoekrryvstinner
authored
[3.15] gh-157335: Fix out-of-bounds write in mmap.mmap.__setitem__ (GH-157438) (#158026)
Co-authored-by: Joseph Kerry <joerkerry@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
1 parent 81330b8 commit 960f69c

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
@@ -74,7 +74,7 @@ def test_basic(self):
7474

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

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

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

16621661
if (!is_writable(self))
16631662
return -1;
16641663

16651664
if (PyIndex_Check(item)) {
16661665
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
1667-
Py_ssize_t v;
1668-
16691666
if (i == -1 && PyErr_Occurred())
16701667
return -1;
1671-
if (i < 0)
1672-
i += self->size;
1673-
if (i < 0 || i >= self->size) {
1674-
PyErr_SetString(PyExc_IndexError,
1675-
"mmap index out of range");
1676-
return -1;
1677-
}
1668+
16781669
if (value == NULL) {
16791670
PyErr_SetString(PyExc_TypeError,
16801671
"mmap doesn't support item deletion");
@@ -1685,7 +1676,7 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16851676
"mmap item value must be an int");
16861677
return -1;
16871678
}
1688-
v = PyNumber_AsSsize_t(value, PyExc_TypeError);
1679+
Py_ssize_t v = PyNumber_AsSsize_t(value, PyExc_TypeError);
16891680
if (v == -1 && PyErr_Occurred())
16901681
return -1;
16911682
if (v < 0 || v > 255) {
@@ -1694,7 +1685,18 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16941685
"in range(0, 256)");
16951686
return -1;
16961687
}
1688+
1689+
/* Converting item or value above may have run arbitrary code
1690+
* (e.g. __index__) that resized or closed the mmap, so bounds
1691+
* are only checked now, against the current size. */
16971692
CHECK_VALID(-1);
1693+
if (i < 0)
1694+
i += self->size;
1695+
if (i < 0 || i >= self->size) {
1696+
PyErr_SetString(PyExc_IndexError,
1697+
"mmap index out of range");
1698+
return -1;
1699+
}
16981700

16991701
char v_char = (char) v;
17001702
if (safe_byte_copy(self->data + i, &v_char) < 0) {
@@ -1709,22 +1711,26 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
17091711
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
17101712
return -1;
17111713
}
1712-
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
17131714
if (value == NULL) {
17141715
PyErr_SetString(PyExc_TypeError,
17151716
"mmap object doesn't support slice deletion");
17161717
return -1;
17171718
}
17181719
if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0)
17191720
return -1;
1721+
1722+
/* Acquiring the buffer above may have run arbitrary code (e.g. a
1723+
* __buffer__ method) that resized or closed this mmap, so the slice bounds
1724+
* are only computed now, against the current size. */
1725+
CHECK_VALID_OR_RELEASE(-1, vbuf);
1726+
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
17201727
if (vbuf.len != slicelen) {
17211728
PyErr_SetString(PyExc_IndexError,
17221729
"mmap slice assignment is wrong size");
17231730
PyBuffer_Release(&vbuf);
17241731
return -1;
17251732
}
17261733

1727-
CHECK_VALID_OR_RELEASE(-1, vbuf);
17281734
int result = 0;
17291735
if (slicelen == 0) {
17301736
}

0 commit comments

Comments
 (0)