Skip to content

Commit ee21d99

Browse files
authored
gh-153570: Fix use-after-free in bytearray.take_bytes() with a reentrant __index__ (#153572)
bytearray.take_bytes() cached the size before the argument's __index__ call and used it for the bounds check and the buffer reads, so an __index__ that resizes the bytearray left it reading freed memory. Re-read the size after __index__, matching bytearray.__setitem__ (GH-91153).
1 parent 0d71cc4 commit ee21d99

3 files changed

Lines changed: 33 additions & 0 deletions

File tree

Lib/test/test_bytes.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,6 +1637,35 @@ def test_take_bytes_optimization(self):
16371637
bytes_header_size = sys.getsizeof(b'')
16381638
self.assertEqual(ba.__alloc__(), 499 + bytes_header_size)
16391639

1640+
def test_take_bytes_reentrant_resize(self):
1641+
# gh-153570: n.__index__() can resize the bytearray, so take_bytes()
1642+
# must re-read the size afterwards. It cached the size before the
1643+
# call and used it for the bounds check and the buffer reads, so a
1644+
# reentrant clear() returned freed memory (a use-after-free read).
1645+
def take(target, resize, n):
1646+
class Evil:
1647+
def __index__(self):
1648+
resize(target)
1649+
return n
1650+
return target.take_bytes(Evil())
1651+
1652+
# clear() during __index__: nothing is left to take.
1653+
ba = bytearray(b'abcdefgh')
1654+
with self.assertRaises(IndexError):
1655+
take(ba, lambda b: b.clear(), 8)
1656+
self.assertEqual(ba, b'')
1657+
1658+
# shrink during __index__: n past the new size is out of range.
1659+
ba = bytearray(b'abcdefgh')
1660+
with self.assertRaises(IndexError):
1661+
take(ba, lambda b: b.__delitem__(slice(4, None)), 8)
1662+
self.assertEqual(ba, b'abcd')
1663+
1664+
# grow during __index__: the take runs against the new, larger size.
1665+
ba = bytearray(b'abcd')
1666+
self.assertEqual(take(ba, lambda b: b.extend(b'efgh'), 8), b'abcdefgh')
1667+
self.assertEqual(ba, b'')
1668+
16401669
def test_setitem(self):
16411670
def setitem_as_mapping(b, i, val):
16421671
b[i] = val
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a use-after-free in :meth:`bytearray.take_bytes` when the argument's
2+
:meth:`~object.__index__` method resizes the bytearray. Patch by tonghuaroot.

Objects/bytearrayobject.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,6 +1555,8 @@ bytearray_take_bytes_impl(PyByteArrayObject *self, PyObject *n)
15551555
if (to_take == -1 && PyErr_Occurred()) {
15561556
return NULL;
15571557
}
1558+
// n.__index__() may have resized self; use the current size.
1559+
size = Py_SIZE(self);
15581560
if (to_take < 0) {
15591561
to_take += size;
15601562
}

0 commit comments

Comments
 (0)