Skip to content

Commit ad18d48

Browse files
committed
Add check on more *_setitem methods
1 parent 67e98b5 commit ad18d48

File tree

2 files changed

+117
-25
lines changed

2 files changed

+117
-25
lines changed

Lib/test/test_array.py

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,32 +1683,44 @@ def test_gh_128961(self):
16831683
def test_gh_142555(self):
16841684
# Test for null pointer dereference in array.__setitem__
16851685
# via re-entrant __index__.
1686-
victim = array.array('b', [0] * 64)
16871686

1688-
class EvilIndex:
1689-
def __index__(self):
1690-
# Re-entrant mutation: shrink the array while __setitem__
1691-
# still holds a pointer to the pre-clear buffer.
1692-
victim.clear()
1693-
return 0
1694-
1695-
with self.assertRaises(IndexError):
1696-
victim[1] = EvilIndex()
1697-
1698-
self.assertEqual(len(victim), 0)
1699-
1700-
# Test case where array is shrunk but not completely cleared
1701-
victim2 = array.array('b', [1, 2, 3])
1702-
1703-
class ShrinkIndex:
1704-
def __index__(self):
1705-
# Pop two elements, making array size 1, so index 1 is out of bounds
1706-
victim2.pop()
1707-
victim2.pop()
1708-
return 0
1709-
1710-
with self.assertRaises(IndexError):
1711-
victim2[1] = ShrinkIndex()
1687+
def test_clear_array(victim):
1688+
"""Test array clearing scenario"""
1689+
class EvilIndex:
1690+
def __index__(self):
1691+
# Re-entrant mutation: clear the array while __setitem__
1692+
# still holds a pointer to the pre-clear buffer.
1693+
victim.clear()
1694+
return 0
1695+
1696+
with self.assertRaises(IndexError):
1697+
victim[1] = EvilIndex()
1698+
1699+
self.assertEqual(len(victim), 0)
1700+
1701+
def test_shrink_array(victim):
1702+
"""Test array shrinking scenario"""
1703+
class ShrinkIndex:
1704+
def __index__(self):
1705+
# Pop two elements, making array size 1, so index 1 is out of bounds
1706+
victim.pop()
1707+
victim.pop()
1708+
return 0
1709+
1710+
with self.assertRaises(IndexError):
1711+
victim[1] = ShrinkIndex() # Original index 1 should now be out of bounds
1712+
1713+
# Test various array types
1714+
test_clear_array(array.array('b', [0] * 64))
1715+
test_shrink_array(array.array('b', [1, 2, 3]))
1716+
test_clear_array(array.array('B', [1, 2, 3]))
1717+
test_clear_array(array.array('h', [1, 2, 3]))
1718+
test_clear_array(array.array('H', [1, 2, 3]))
1719+
test_clear_array(array.array('i', [1, 2, 3]))
1720+
test_clear_array(array.array('l', [1, 2, 3]))
1721+
test_clear_array(array.array('q', [1, 2, 3]))
1722+
test_clear_array(array.array('f', [1.0, 2.0, 3.0]))
1723+
test_clear_array(array.array('d', [1.0, 2.0, 3.0]))
17121724

17131725

17141726
if __name__ == "__main__":

Modules/arraymodule.c

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,16 @@ BB_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
260260
/* 'B' == unsigned char, maps to PyArg_Parse's 'b' formatter */
261261
if (!PyArg_Parse(v, "b;array item must be integer", &x))
262262
return -1;
263+
264+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
265+
* __index__ on v, which might modify the array buffer. See gh-142555.
266+
*/
267+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
268+
PyErr_SetString(PyExc_IndexError,
269+
"array assignment index out of range");
270+
return -1;
271+
}
272+
263273
if (i >= 0)
264274
((unsigned char *)ap->ob_item)[i] = x;
265275
return 0;
@@ -352,6 +362,16 @@ h_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
352362
/* 'h' == signed short, maps to PyArg_Parse's 'h' formatter */
353363
if (!PyArg_Parse(v, "h;array item must be integer", &x))
354364
return -1;
365+
366+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
367+
* __index__ on v, which might modify the array buffer. See gh-142555.
368+
*/
369+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
370+
PyErr_SetString(PyExc_IndexError,
371+
"array assignment index out of range");
372+
return -1;
373+
}
374+
355375
if (i >= 0)
356376
((short *)ap->ob_item)[i] = x;
357377
return 0;
@@ -381,6 +401,16 @@ HH_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
381401
"unsigned short is greater than maximum");
382402
return -1;
383403
}
404+
405+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
406+
* __index__ on v, which might modify the array buffer. See gh-142555.
407+
*/
408+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
409+
PyErr_SetString(PyExc_IndexError,
410+
"array assignment index out of range");
411+
return -1;
412+
}
413+
384414
if (i >= 0)
385415
((short *)ap->ob_item)[i] = (short)x;
386416
return 0;
@@ -399,6 +429,16 @@ i_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
399429
/* 'i' == signed int, maps to PyArg_Parse's 'i' formatter */
400430
if (!PyArg_Parse(v, "i;array item must be integer", &x))
401431
return -1;
432+
433+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
434+
* __index__ on v, which might modify the array buffer. See gh-142555.
435+
*/
436+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
437+
PyErr_SetString(PyExc_IndexError,
438+
"array assignment index out of range");
439+
return -1;
440+
}
441+
402442
if (i >= 0)
403443
((int *)ap->ob_item)[i] = x;
404444
return 0;
@@ -460,6 +500,16 @@ l_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
460500
long x;
461501
if (!PyArg_Parse(v, "l;array item must be integer", &x))
462502
return -1;
503+
504+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
505+
* __index__ on v, which might modify the array buffer. See gh-142555.
506+
*/
507+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
508+
PyErr_SetString(PyExc_IndexError,
509+
"array assignment index out of range");
510+
return -1;
511+
}
512+
463513
if (i >= 0)
464514
((long *)ap->ob_item)[i] = x;
465515
return 0;
@@ -512,6 +562,16 @@ q_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
512562
long long x;
513563
if (!PyArg_Parse(v, "L;array item must be integer", &x))
514564
return -1;
565+
566+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
567+
* __index__ on v, which might modify the array buffer. See gh-142555.
568+
*/
569+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
570+
PyErr_SetString(PyExc_IndexError,
571+
"array assignment index out of range");
572+
return -1;
573+
}
574+
515575
if (i >= 0)
516576
((long long *)ap->ob_item)[i] = x;
517577
return 0;
@@ -565,6 +625,16 @@ f_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
565625
float x;
566626
if (!PyArg_Parse(v, "f;array item must be float", &x))
567627
return -1;
628+
629+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
630+
* __index__ on v, which might modify the array buffer. See gh-142555.
631+
*/
632+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
633+
PyErr_SetString(PyExc_IndexError,
634+
"array assignment index out of range");
635+
return -1;
636+
}
637+
568638
if (i >= 0)
569639
((float *)ap->ob_item)[i] = x;
570640
return 0;
@@ -582,6 +652,16 @@ d_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
582652
double x;
583653
if (!PyArg_Parse(v, "d;array item must be float", &x))
584654
return -1;
655+
656+
/* Check buffer validity and bounds after PyArg_Parse which may call user-defined
657+
* __index__ on v, which might modify the array buffer. See gh-142555.
658+
*/
659+
if (i >= 0 && (ap->ob_item == NULL || i >= Py_SIZE(ap))) {
660+
PyErr_SetString(PyExc_IndexError,
661+
"array assignment index out of range");
662+
return -1;
663+
}
664+
585665
if (i >= 0)
586666
((double *)ap->ob_item)[i] = x;
587667
return 0;

0 commit comments

Comments
 (0)