Skip to content

Commit c554143

Browse files
miss-islingtonJoekrryvstinner
authored
[3.14] gh-157335: Fix out-of-bounds write in mmap.mmap.__setitem__ (GH-157438) (#158027)
gh-157335: Fix out-of-bounds write in mmap.mmap.__setitem__ (GH-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> Co-authored-by: Victor Stinner <vstinner@python.org>
1 parent 9b568a2 commit c554143

3 files changed

Lines changed: 73 additions & 14 deletions

File tree

‎Lib/test/test_mmap.py‎

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

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

7777
# Modify the file's content
7878
m[0] = b'3'[0]
@@ -974,6 +974,55 @@ def test_resize_down_anonymous_mapping(self):
974974
with self.assertRaises(ValueError):
975975
m.resize(start_size)
976976

977+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
978+
def test_setitem_resize_reentrancy(self):
979+
"""Resizing the mmap from inside __index__ while assigning to a
980+
single item must not access memory past the new bounds (gh-157335).
981+
"""
982+
size = 2 * PAGESIZE
983+
new_size = PAGESIZE
984+
985+
class ResizeOnIndex:
986+
def __init__(self, m):
987+
self.m = m
988+
def __index__(self):
989+
self.m.resize(new_size)
990+
return 0
991+
992+
with mmap.mmap(-1, size) as m:
993+
try:
994+
with self.assertRaises(IndexError):
995+
m[size - 1] = ResizeOnIndex(m)
996+
except SystemError as exc:
997+
self.skipTest(f"resize() is not available: {exc!r}")
998+
self.assertEqual(len(m), new_size)
999+
1000+
@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
1001+
def test_setitem_slice_resize_reentrancy(self):
1002+
"""Resizing the mmap from inside a value's buffer-protocol
1003+
callback while assigning to a slice must not access memory past
1004+
the new bounds (gh-157335).
1005+
"""
1006+
size = 2 * PAGESIZE
1007+
new_size = PAGESIZE
1008+
1009+
class ResizeOnBuffer:
1010+
def __init__(self, m, data):
1011+
self.m = m
1012+
self.data = data
1013+
def __buffer__(self, flags):
1014+
self.m.resize(new_size)
1015+
return memoryview(self.data)
1016+
1017+
with mmap.mmap(-1, size) as m:
1018+
value = ResizeOnBuffer(m, bytes(size))
1019+
try:
1020+
with self.assertRaises(IndexError):
1021+
m[0:size] = value
1022+
except SystemError as exc:
1023+
self.skipTest(f"resize() is not available: {exc!r}")
1024+
self.assertEqual(len(m), new_size)
1025+
9771026
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
9781027
def test_resize_fails_if_mapping_held_elsewhere(self):
9791028
"""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
@@ -1608,24 +1608,15 @@ static int
16081608
mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16091609
{
16101610
mmap_object *self = mmap_object_CAST(op);
1611-
CHECK_VALID(-1);
16121611

16131612
if (!is_writable(self))
16141613
return -1;
16151614

16161615
if (PyIndex_Check(item)) {
16171616
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
1618-
Py_ssize_t v;
1619-
16201617
if (i == -1 && PyErr_Occurred())
16211618
return -1;
1622-
if (i < 0)
1623-
i += self->size;
1624-
if (i < 0 || i >= self->size) {
1625-
PyErr_SetString(PyExc_IndexError,
1626-
"mmap index out of range");
1627-
return -1;
1628-
}
1619+
16291620
if (value == NULL) {
16301621
PyErr_SetString(PyExc_TypeError,
16311622
"mmap doesn't support item deletion");
@@ -1636,7 +1627,7 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16361627
"mmap item value must be an int");
16371628
return -1;
16381629
}
1639-
v = PyNumber_AsSsize_t(value, PyExc_TypeError);
1630+
Py_ssize_t v = PyNumber_AsSsize_t(value, PyExc_TypeError);
16401631
if (v == -1 && PyErr_Occurred())
16411632
return -1;
16421633
if (v < 0 || v > 255) {
@@ -1645,7 +1636,18 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16451636
"in range(0, 256)");
16461637
return -1;
16471638
}
1639+
1640+
/* Converting item or value above may have run arbitrary code
1641+
* (e.g. __index__) that resized or closed the mmap, so bounds
1642+
* are only checked now, against the current size. */
16481643
CHECK_VALID(-1);
1644+
if (i < 0)
1645+
i += self->size;
1646+
if (i < 0 || i >= self->size) {
1647+
PyErr_SetString(PyExc_IndexError,
1648+
"mmap index out of range");
1649+
return -1;
1650+
}
16491651

16501652
char v_char = (char) v;
16511653
if (safe_byte_copy(self->data + i, &v_char) < 0) {
@@ -1660,22 +1662,26 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
16601662
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
16611663
return -1;
16621664
}
1663-
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
16641665
if (value == NULL) {
16651666
PyErr_SetString(PyExc_TypeError,
16661667
"mmap object doesn't support slice deletion");
16671668
return -1;
16681669
}
16691670
if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0)
16701671
return -1;
1672+
1673+
/* Acquiring the buffer above may have run arbitrary code (e.g. a
1674+
* __buffer__ method) that resized or closed this mmap, so the slice bounds
1675+
* are only computed now, against the current size. */
1676+
CHECK_VALID_OR_RELEASE(-1, vbuf);
1677+
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
16711678
if (vbuf.len != slicelen) {
16721679
PyErr_SetString(PyExc_IndexError,
16731680
"mmap slice assignment is wrong size");
16741681
PyBuffer_Release(&vbuf);
16751682
return -1;
16761683
}
16771684

1678-
CHECK_VALID_OR_RELEASE(-1, vbuf);
16791685
int result = 0;
16801686
if (slicelen == 0) {
16811687
}

0 commit comments

Comments
 (0)