Skip to content
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.

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

gh-120384: Fix array-out-of-bounds crash in list_ass_subscript #120442

Merged
merged 6 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions Lib/test/list_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,14 @@ def test_setslice(self):

self.assertRaises(TypeError, a.__setitem__)

def test_slice_assign_iterator(self):
x = self.type2test(range(5))
x[0:3] = reversed(range(3))
self.assertEqual(x, self.type2test([2, 1, 0, 3, 4]))

x[:] = reversed(range(3))
self.assertEqual(x, self.type2test([2, 1, 0]))

def test_delslice(self):
a = self.type2test([0, 1])
del a[1:2]
Expand Down
14 changes: 14 additions & 0 deletions Lib/test/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,20 @@ def __lt__(self, other):
with self.assertRaises(TypeError):
a[0] < a

def test_list_index_modifing_operand(self):
# See gh-120384
class evil:
def __init__(self, lst):
self.lst = lst
def __iter__(self):
yield from self.lst
self.lst.clear()

lst = list(range(5))
operand = evil(lst)
with self.assertRaises(ValueError):
lst[::-1] = operand

@cpython_only
def test_preallocation(self):
iterable = [0] * 10
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix an array out of bounds crash in ``list_ass_subscript``, which could be
invoked via some specificly tailored input: including concurrent modification
of a list object, where one thread assigns a slice and another clears it.
45 changes: 33 additions & 12 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3581,6 +3581,23 @@ list_subscript(PyObject* _self, PyObject* item)
}
}

static Py_ssize_t
adjust_slice_indexes(PyListObject *lst,
Py_ssize_t *start, Py_ssize_t *stop,
Py_ssize_t step)
{
Py_ssize_t slicelength = PySlice_AdjustIndices(Py_SIZE(lst), start, stop,
step);

/* Make sure s[5:2] = [..] inserts at the right place:
before 5, not before 2. */
if ((step < 0 && *start < *stop) ||
(step > 0 && *start > *stop))
*stop = *start;

return slicelength;
}

static int
list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
{
Expand All @@ -3594,22 +3611,11 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
return list_ass_item((PyObject *)self, i, value);
}
else if (PySlice_Check(item)) {
Py_ssize_t start, stop, step, slicelength;
Py_ssize_t start, stop, step;

if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
return -1;
}
slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop,
step);

if (step == 1)
return list_ass_slice(self, start, stop, value);

/* Make sure s[5:2] = [..] inserts at the right place:
before 5, not before 2. */
if ((step < 0 && start < stop) ||
(step > 0 && start > stop))
stop = start;

if (value == NULL) {
/* delete slice */
Expand All @@ -3618,6 +3624,12 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
Py_ssize_t i;
int res;

Py_ssize_t slicelength = adjust_slice_indexes(self, &start, &stop,
step);

if (step == 1)
return list_ass_slice(self, start, stop, value);

if (slicelength <= 0)
return 0;

Expand Down Expand Up @@ -3695,6 +3707,15 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
if (!seq)
return -1;

Py_ssize_t slicelength = adjust_slice_indexes(self, &start, &stop,
step);

if (step == 1) {
int res = list_ass_slice(self, start, stop, seq);
Py_DECREF(seq);
return res;
}

if (PySequence_Fast_GET_SIZE(seq) != slicelength) {
PyErr_Format(PyExc_ValueError,
"attempt to assign sequence of "
Expand Down
Loading