Array out of bounds assignment in list_ass_subscript

[MechanicPig]

Crash report

What happened?

Root Cause

When step is not 1 in slice assignment, list_ass_subscript first calculates the length of the slice and then converts the input iterable into a list. During the conversion, arbitrary code in Python can be executed to modify the length of the current list or even clear it:

/* Python 3.10 source code */

static int
list_ass_subscript(PyListObject* self, PyObject* item, PyObject* value)
{
 if (_PyIndex_Check(item)){/* ... */ }
 else if (PySlice_Check(item)){
 Py_ssize_t start, stop, step, slicelength;

 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){/* ... */ }
 else{
 /* assign slice */
 PyObject *ins, *seq;
 PyObject **garbage, **seqitems, **selfitems;
 Py_ssize_t i;
 size_t cur;

 /* protect against a[::-1] = a */
 if (self == (PyListObject*)value){
 seq = list_slice((PyListObject*)value, 0,
 PyList_GET_SIZE(value));
 }
 else{
 seq = PySequence_Fast(value, // <-- call arbitrary code in python
 "must assign iterable "
 "to extended slice");
 }
 if (!seq)
 return -1;
 /* ... */
 selfitems = self->ob_item;
 seqitems = PySequence_Fast_ITEMS(seq);
 for (cur = start, i = 0; i < slicelength;
 cur += (size_t)step, i++){
 garbage[i] = selfitems[cur];
 ins = seqitems[i];
 Py_INCREF(ins);
 selfitems[cur] = ins; // <-- maybe out of bounds
 }
 /* ... */
 }
 /* ... */
}

POC

class evil:
 def __init__(self, lst):
 self.lst = lst
 def __iter__(self):
 yield from self.lst
 self.lst.clear()


lst = list(range(10))
lst[::-1] = evil(lst)

CPython versions tested on:

3.10, 3.11, 3.12

Operating systems tested on:

Windows

Output from running 'python -VV' on the command line:

Python 3.10.11 (tags/v3.10.11:7d4cc5a, Apr 5 2023, 00:38:17) [MSC v.1929 64 bit (AMD64)]
Python 3.11.8 (tags/v3.11.8:db85d51, Feb 6 2024, 22:03:32) [MSC v.1937 64 bit (AMD64)]
Python 3.12.3 (tags/v3.12.3:f6650f9, Apr 9 2024, 14:05:25) [MSC v.1938 64 bit (AMD64)]

Linked PRs

• gh-120384: Fix array-out-of-bounds crash in list_ass_subscript #120442
• [3.12] gh-120384: Fix array-out-of-bounds crash in list_ass_subscript (GH-120442) #120825
• [3.13] gh-120384: Fix array-out-of-bounds crash in list_ass_subscript (GH-120442) #120826
• [3.11] gh-120384: gh-120298: Fix array-out-of-bounds & use after free list #121345
• [3.10] gh-120384: gh-120298: Fix array-out-of-bounds & use after free list (GH-121345) #140833
• [3.9] gh-120384: gh-120298: Fix array-out-of-bounds & use after free list (GH-121345) #140834

[Eclips4]

Thanks for the report! Confirmed on current main.

[sobolevn]

Hm, I have an idea about a potential solution, but it is not very straight-forward.

Result with my patch:

» ./python.exe ex.py
Traceback (most recent call last):
 File "/Users/sobolev/Desktop/cpython2/ex.py", line 10, in <module>
 lst[::-1] = evil(lst)
 ~~~^^^^^^
ValueError: attempt to assign sequence of size 10 to extended slice of size 0 

What I did?

1. I removed this part:

   cpython/Objects/listobject.c

   Lines 3597 to 3612 in 19435d2

   	Py_ssize_t start, stop, step, slicelength;
   	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;

2. I moved PySlice_AdjustIndices calls after the PySequence_Fast call
3. I also added the same call to value == NULL case

What still needs to be done? TODO:

• This looks quite ugly now, probably a new function is required

Diff:

diff --git Objects/listobject.c Objects/listobject.c
index 6829d5d2865..2a32cec19e9 100644
--- Objects/listobject.c
+++ Objects/listobject.c
@@ -3594,23 +3594,6 @@ 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;
-
- 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 */
 PyObject **garbage;
@@ -3618,6 +3601,23 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
 Py_ssize_t i;
 int res;
 
+ Py_ssize_t start, stop, step, slicelength;
+
+ 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 (slicelength <= 0)
 return 0;
 
@@ -3695,6 +3695,25 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
 if (!seq)
 return -1;
 
+ Py_ssize_t start, stop, step, slicelength;
+
+ if (PySlice_Unpack(item, &start, &stop, &step) < 0){
+ return -1;
+ }
+ slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop,
+ step);
+
+ if (step == 1){
+ Py_DECREF(seq);
+ 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 (PySequence_Fast_GET_SIZE(seq) != slicelength){
 PyErr_Format(PyExc_ValueError,
 "attempt to assign sequence of "
 

What do others think? Does this seem like a solution? (please, note the TODO item)

[sobolevn]

cc @serhiy-storchaka @vstinner

[serhiy-storchaka]

PySlice_GetIndicesEx() was split on two steps PySlice_Unpack() and PySlice_AdjustIndices() to avoid a similar problem -- using the old value of Py_SIZE(self) which can be invalidated during calling custom __index__() method.

I think that moving PySlice_AdjustIndices() is the right solution, but you can keep PySlice_Unpack() on its current place.