8000 gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods by bast0006 · Pull Request #132379 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods #132379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 12, 2025
Merged

gh-91153: Fix bytearray holding a reference to its internal buffer when calling into potentially mutating __index__ methods #132379

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
71ae212
GH-91153: Handle _getbytevalue potentially invalidating bytearray all…
bast0006 Mar 29, 2025
b2f7df5
GH-91153: Add additional cases to `test_mutating_index`
bast0006 Apr 10, 2025
bb96c2a
GH-91153: Move mutating index bytearray test to it's own separate test
bast0006 May 17, 2025
d29422b
GH-91153: add news entry
bast0006 May 17, 2025
d3d1974
Nit cleanup
bast0006 May 18, 2025
18e6597
GH-91153: Fixup comments and NEWS entry
bast0006 Jun 3, 2025
4b7ec9c
GH-91153: Add short clarifying comments to test
bast0006 Jun 3, 2025
9b28503
Merge branch 'main' into bast0006/gh-91153
picnixz Jul 12, 2025
970c1ad
Apply suggestions from code review
picnixz Jul 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions Lib/test/test_bytes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,44 @@ def __index__(self):
with self.assertRaises(IndexError):
self._testlimitedcapi.sequence_setitem(b, 0, Boom())

def test_mutating_index_inbounds(self):
class MutatesOnIndex:
new_ba: bytearray

def __init__(self):
self.ba = bytearray(0x180)

def __index__(self):
# Clear the original bytearray, mutating it during index assignment.
# If the internal buffers are held over this operation, they become dangling
# However, this will fail a bounds check as above (as the clear sets bounds to zero)
self.ba.clear()
# At this moment, the bytearray potentially has a dangling pointer
# Create a new bytearray to catch any writes
self.new_ba = bytearray(0x180)
# Ensure bounds check passes
self.ba.extend([0] * 0x180)
return 0

with self.subTest("skip_bounds_safety"):
instance = MutatesOnIndex()
instance.ba[instance] = ord("?")
self.assertEqual(instance.ba[0], ord("?"), "Assigned bytearray not altered")
self.assertEqual(instance.new_ba, bytearray(0x180), "Wrong object altered")

with self.subTest("skip_bounds_safety_capi"):
instance = MutatesOnIndex()
instance.ba[instance] = ord("?")
self._testlimitedcapi.sequence_setitem(instance.ba, instance, ord("?"))
self.assertEqual(instance.ba[0], ord("?"), "Assigned bytearray not altered")
self.assertEqual(instance.new_ba, bytearray(0x180), "Wrong object altered")

with self.subTest("skip_bounds_safety_slice"):
instance = MutatesOnIndex()
instance.ba[instance:1] = [ord("?")]
self.assertEqual(instance.ba[0], ord("?"), "Assigned bytearray not altered")
self.assertEqual(instance.new_ba, bytearray(0x180), "Wrong object altered")


class AssortedBytesTest(unittest.TestCase):
#
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-05-17-18-22-12.gh-issue-91153.ioA_83.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix an issue where a :class:`bytearray` item assignment could crash or write
to the wrong bytearray when resized by the new value's :meth:`__index__`
method.
7 changes: 5 additions & 2 deletions Objects/bytearrayobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,8 @@ bytearray_ass_subscript_lock_held(PyObject *op, PyObject *index, PyObject *value
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
PyByteArrayObject *self = _PyByteArray_CAST(op);
Py_ssize_t start, stop, step, slicelen;
char *buf = PyByteArray_AS_STRING(self);
// GH-91153: we cannot store a reference to the internal buffer here, as _getbytevalue might call into python code
// that could then invalidate it.

if (_PyIndex_Check(index)) {
Py_ssize_t i = PyNumber_AsSsize_t(index, PyExc_IndexError);
Expand Down Expand Up @@ -744,7 +745,7 @@ bytearray_ass_subscript_lock_held(PyObject *op, PyObject *index, PyObject *value
}
else {
assert(0 <= ival && ival < 256);
buf[i] = (char)ival;
PyByteArray_AS_STRING(self)[i] = (char)ival;
return 0;
}
}
Expand Down Expand Up @@ -805,6 +806,7 @@ bytearray_ass_subscript_lock_held(PyObject *op, PyObject *index, PyObject *value
/* Delete slice */
size_t cur;
Py_ssize_t i;
char* buf = PyByteArray_AS_STRING(self);

if (!_canresize(self))
return -1;
Expand Down Expand Up @@ -845,6 +847,7 @@ bytearray_ass_subscript_lock_held(PyObject *op, PyObject *index, PyObject *value
/* Assign slice */
Py_ssize_t i;
size_t cur;
char* buf = PyByteArray_AS_STRING(self);

if (needed != slicelen) {
PyErr_Format(PyExc_ValueError,
Expand Down
Loading
0