Skip to content

Commit bd72783

Browse files
committed
pythongh-142884: Fix use-after-free in array.array.tofile() with reentrant writer
array_array_tofile_impl() pre-computed nbytes and nblocks once at the start of the function. If the file-like object's write() callback mutated the array (e.g. by clearing it or replacing its contents), the cached values became stale and subsequent iterations read from freed or invalid memory. Fix by re-checking Py_SIZE(self) on every loop iteration so the loop terminates safely when the array is modified during the write callback.
1 parent 6577d87 commit bd72783

3 files changed

Lines changed: 68 additions & 10 deletions

File tree

Lib/test/test_array.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,58 @@ def __float__(self):
17371737
self.assertRaises(IndexError, victim.__setitem__, 1, Float())
17381738
self.assertEqual(len(victim), 0)
17391739

1740+
# Tests for use-after-free in array.tofile() when the writer
1741+
# callback mutates the array.
1742+
# See: https://github.com/python/cpython/issues/142884.
1743+
1744+
def test_tofile_reentrant_write_clear(self):
1745+
# tofile() must not crash when f.write() clears the array.
1746+
# Needs >64 KB so tofile() uses multiple blocks.
1747+
BLOCKSIZE = 64 * 1024
1748+
victim = array.array('B', b'\0' * (BLOCKSIZE * 2))
1749+
1750+
class Writer:
1751+
armed = True
1752+
def write(self, data):
1753+
if Writer.armed:
1754+
Writer.armed = False
1755+
victim.clear()
1756+
return len(data)
1757+
1758+
victim.tofile(Writer()) # must not crash
1759+
1760+
def test_tofile_reentrant_write_shrink(self):
1761+
# tofile() must not crash when f.write() shrinks the array.
1762+
BLOCKSIZE = 64 * 1024
1763+
victim = array.array('B', b'\0' * (BLOCKSIZE * 2))
1764+
1765+
class Writer:
1766+
armed = True
1767+
def write(self, data):
1768+
if Writer.armed:
1769+
Writer.armed = False
1770+
victim[:] = array.array('B', b'\0')
1771+
return len(data)
1772+
1773+
victim.tofile(Writer()) # must not crash
1774+
1775+
def test_tofile_reentrant_write_reallocate(self):
1776+
# tofile() must not crash when f.write() clears and
1777+
# reallocates the array to a smaller buffer.
1778+
BLOCKSIZE = 64 * 1024
1779+
victim = array.array('B', b'\0' * (BLOCKSIZE * 2))
1780+
1781+
class Writer:
1782+
armed = True
1783+
def write(self, data):
1784+
if Writer.armed:
1785+
Writer.armed = False
1786+
victim.clear()
1787+
victim.append(0)
1788+
return len(data)
1789+
1790+
victim.tofile(Writer()) # must not crash
1791+
17401792

17411793
if __name__ == "__main__":
17421794
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix crash in :meth:`array.array.tofile` when a reentrant ``write()``
2+
callback mutates the array. The function now re-checks the array size on
3+
every iteration instead of caching it once at the start.

Modules/arraymodule.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,31 +1659,34 @@ static PyObject *
16591659
array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f)
16601660
/*[clinic end generated code: output=4560c628d9c18bc2 input=5a24da7a7b407b52]*/
16611661
{
1662-
Py_ssize_t nbytes = Py_SIZE(self) * self->ob_descr->itemsize;
16631662
/* Write 64K blocks at a time */
16641663
/* XXX Make the block size settable */
16651664
int BLOCKSIZE = 64*1024;
1666-
Py_ssize_t nblocks = (nbytes + BLOCKSIZE - 1) / BLOCKSIZE;
16671665
Py_ssize_t i;
16681666

16691667
if (Py_SIZE(self) == 0)
16701668
goto done;
16711669

1672-
16731670
array_state *state = get_array_state_by_class(cls);
16741671
assert(state != NULL);
16751672

1676-
for (i = 0; i < nblocks; i++) {
1677-
char* ptr = self->ob_item + i*BLOCKSIZE;
1673+
/* Re-check Py_SIZE() on every iteration because f.write() could
1674+
execute arbitrary Python code that modifies or clears the array. */
1675+
for (i = 0; ; i++) {
1676+
Py_ssize_t nbytes = Py_SIZE(self) * self->ob_descr->itemsize;
1677+
Py_ssize_t offset = (Py_ssize_t)i * BLOCKSIZE;
1678+
if (offset >= nbytes)
1679+
break;
1680+
16781681
Py_ssize_t size = BLOCKSIZE;
1679-
PyObject *bytes, *res;
1682+
if (offset + size > nbytes)
1683+
size = nbytes - offset;
16801684

1681-
if (i*BLOCKSIZE + size > nbytes)
1682-
size = nbytes - i*BLOCKSIZE;
1683-
bytes = PyBytes_FromStringAndSize(ptr, size);
1685+
char *ptr = self->ob_item + offset;
1686+
PyObject *bytes = PyBytes_FromStringAndSize(ptr, size);
16841687
if (bytes == NULL)
16851688
return NULL;
1686-
res = PyObject_CallMethodOneArg(f, state->str_write, bytes);
1689+
PyObject *res = PyObject_CallMethodOneArg(f, state->str_write, bytes);
16871690
Py_DECREF(bytes);
16881691
if (res == NULL)
16891692
return NULL;

0 commit comments

Comments
 (0)