From 14514d9084a40f599c57da853a305aa264562a43 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Fri, 17 May 2019 01:13:03 -0600 Subject: [PATCH] bpo-36946: Fix possible signed integer overflow when handling slices. (GH-13375) The final addition (cur += step) may overflow, so use size_t for "cur". "cur" is always positive (even for negative steps), so it is safe to use size_t here. Co-Authored-By: Martin Panter --- Lib/ctypes/test/test_arrays.py | 11 +++++++++++ Lib/test/seq_tests.py | 1 + Lib/test/string_tests.py | 2 +- Lib/test/test_array.py | 4 ++-- Lib/test/test_bytes.py | 5 +++-- Lib/test/test_mmap.py | 4 ++-- .../2019-05-16-23-53-45.bpo-36946.qjxr0Y.rst | 1 + Modules/_ctypes/_ctypes.c | 6 ++++-- Modules/_elementtree.c | 6 ++++-- Modules/arraymodule.c | 6 ++++-- Modules/mmapmodule.c | 6 ++++-- Objects/bytearrayobject.c | 3 ++- Objects/bytesobject.c | 3 ++- Objects/tupleobject.c | 3 ++- Objects/unicodeobject.c | 3 ++- 15 files changed, 45 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-05-16-23-53-45.bpo-36946.qjxr0Y.rst diff --git a/Lib/ctypes/test/test_arrays.py b/Lib/ctypes/test/test_arrays.py index 0fc5d7ebf84..87ecbf04e7e 100644 --- a/Lib/ctypes/test/test_arrays.py +++ b/Lib/ctypes/test/test_arrays.py @@ -69,6 +69,17 @@ class ArrayTestCase(unittest.TestCase): from operator import delitem self.assertRaises(TypeError, delitem, ca, 0) + def test_step_overflow(self): + a = (c_int * 5)() + a[3::sys.maxsize] = (1,) + self.assertListEqual(a[3::sys.maxsize], [1]) + a = (c_char * 5)() + a[3::sys.maxsize] = b"A" + self.assertEqual(a[3::sys.maxsize], b"A") + a = (c_wchar * 5)() + a[3::sys.maxsize] = u"X" + self.assertEqual(a[3::sys.maxsize], u"X") + def test_numeric_arrays(self): alen = 5 diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py index 6aedd2be94d..65b110ef781 100644 --- a/Lib/test/seq_tests.py +++ b/Lib/test/seq_tests.py @@ -209,6 +209,7 @@ class CommonTest(unittest.TestCase): a = self.type2test([0,1,2,3,4]) self.assertEqual(a[ -pow(2,128): 3 ], self.type2test([0,1,2])) self.assertEqual(a[ 3: pow(2,145) ], self.type2test([3,4])) + self.assertEqual(a[3::sys.maxsize], self.type2test([3])) def test_contains(self): u = self.type2test([0, 1, 2]) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 836a43b81dd..38da941f7f5 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -1135,7 +1135,7 @@ class MixinStrUnicodeUserStringTest: def test_extended_getslice(self): # Test extended slicing by comparing with list slicing. s = string.ascii_letters + string.digits - indices = (0, None, 1, 3, 41, -1, -2, -37) + indices = (0, None, 1, 3, 41, sys.maxsize, -1, -2, -37) for start in indices: for stop in indices: # Skip step 0 (invalid) diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py index 57c396d610b..c2439579e8e 100644 --- a/Lib/test/test_array.py +++ b/Lib/test/test_array.py @@ -746,7 +746,7 @@ class BaseTest: # Test extended slicing by comparing with list slicing # (Assumes list conversion works correctly, too) a = array.array(self.typecode, self.example) - indices = (0, None, 1, 3, 19, 100, -1, -2, -31, -100) + indices = (0, None, 1, 3, 19, 100, sys.maxsize, -1, -2, -31, -100) for start in indices: for stop in indices: # Everything except the initial 0 (invalid step) @@ -844,7 +844,7 @@ class BaseTest: self.assertRaises(TypeError, a.__setitem__, slice(0, 1), b) def test_extended_set_del_slice(self): - indices = (0, None, 1, 3, 19, 100, -1, -2, -31, -100) + indices = (0, None, 1, 3, 19, 100, sys.maxsize, -1, -2, -31, -100) for start in indices: for stop in indices: # Everything except the initial 0 (invalid step) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index a0913603e73..9502a8f974b 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -285,7 +285,7 @@ class BaseBytesTest: # Test extended slicing by comparing with list slicing. L = list(range(255)) b = self.type2test(L) - indices = (0, None, 1, 3, 19, 100, -1, -2, -31, -100) + indices = (0, None, 1, 3, 19, 100, sys.maxsize, -1, -2, -31, -100) for start in indices: for stop in indices: # Skip step 0 (invalid) @@ -1242,7 +1242,8 @@ class ByteArrayTest(BaseBytesTest, unittest.TestCase): self.assertLessEqual(sys.getsizeof(b), size) def test_extended_set_del_slice(self): - indices = (0, None, 1, 3, 19, 300, 1<<333, -1, -2, -31, -300) + indices = (0, None, 1, 3, 19, 300, 1<<333, sys.maxsize, + -1, -2, -31, -300) for start in indices: for stop in indices: # Skip invalid step 0 diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index dd857a0632a..495d24ad807 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -439,7 +439,7 @@ class MmapTests(unittest.TestCase): m = mmap.mmap(-1, len(s)) m[:] = s self.assertEqual(m[:], s) - indices = (0, None, 1, 3, 19, 300, -1, -2, -31, -300) + indices = (0, None, 1, 3, 19, 300, sys.maxsize, -1, -2, -31, -300) for start in indices: for stop in indices: # Skip step 0 (invalid) @@ -451,7 +451,7 @@ class MmapTests(unittest.TestCase): # Test extended slicing by comparing with list slicing. s = bytes(reversed(range(256))) m = mmap.mmap(-1, len(s)) - indices = (0, None, 1, 3, 19, 300, -1, -2, -31, -300) + indices = (0, None, 1, 3, 19, 300, sys.maxsize, -1, -2, -31, -300) for start in indices: for stop in indices: # Skip invalid step 0 diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-05-16-23-53-45.bpo-36946.qjxr0Y.rst b/Misc/NEWS.d/next/Core and Builtins/2019-05-16-23-53-45.bpo-36946.qjxr0Y.rst new file mode 100644 index 00000000000..aa5de80a294 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-05-16-23-53-45.bpo-36946.qjxr0Y.rst @@ -0,0 +1 @@ +Fix possible signed integer overflow when handling slices. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 13cf76a16d7..7b5115359ed 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -4487,7 +4487,8 @@ Array_subscript(PyObject *myself, PyObject *item) StgDictObject *stgdict, *itemdict; PyObject *proto; PyObject *np; - Py_ssize_t start, stop, step, slicelen, cur, i; + Py_ssize_t start, stop, step, slicelen, i; + size_t cur; if (PySlice_Unpack(item, &start, &stop, &step) < 0) { return NULL; @@ -4627,7 +4628,8 @@ Array_ass_subscript(PyObject *myself, PyObject *item, PyObject *value) return Array_ass_item(myself, i, value); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelen, otherlen, i, cur; + Py_ssize_t start, stop, step, slicelen, otherlen, i; + size_t cur; if (PySlice_Unpack(item, &start, &stop, &step) < 0) { return -1; diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index f5fc4437deb..b1fb3eeffb1 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -1809,7 +1809,8 @@ element_subscr(PyObject* self_, PyObject* item) return element_getitem(self_, i); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelen, cur, i; + Py_ssize_t start, stop, step, slicelen, i; + size_t cur; PyObject* list; if (!self->extra) @@ -1861,7 +1862,8 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) return element_setitem(self_, i, value); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelen, newlen, cur, i; + Py_ssize_t start, stop, step, slicelen, newlen, i; + size_t cur; PyObject* recycle = NULL; PyObject* seq; diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 4be3beb29cc..523afb99e8a 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -2343,7 +2343,8 @@ array_subscr(arrayobject* self, PyObject* item) return array_item(self, i); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelength, cur, i; + Py_ssize_t start, stop, step, slicelength, i; + size_t cur; PyObject* result; arrayobject* ar; int itemsize = self->ob_descr->itemsize; @@ -2527,7 +2528,8 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) return 0; } else { - Py_ssize_t cur, i; + size_t cur; + Py_ssize_t i; if (needed != slicelength) { PyErr_Format(PyExc_ValueError, diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 6ddbf70d9a9..33366b2d933 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -806,7 +806,8 @@ mmap_subscript(mmap_object *self, PyObject *item) slicelen); else { char *result_buf = (char *)PyMem_Malloc(slicelen); - Py_ssize_t cur, i; + size_t cur; + Py_ssize_t i; PyObject *result; if (result_buf == NULL) @@ -926,7 +927,8 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value) memcpy(self->data + start, vbuf.buf, slicelen); } else { - Py_ssize_t cur, i; + size_t cur; + Py_ssize_t i; for (cur = start, i = 0; i < slicelen; diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index eaf5dceb03a..8d5ba540120 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -410,7 +410,8 @@ bytearray_subscript(PyByteArrayObject *self, PyObject *index) return PyLong_FromLong((unsigned char)(PyByteArray_AS_STRING(self)[i])); } else if (PySlice_Check(index)) { - Py_ssize_t start, stop, step, slicelength, cur, i; + Py_ssize_t start, stop, step, slicelength, i; + size_t cur; if (PySlice_Unpack(index, &start, &stop, &step) < 0) { return NULL; } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index b7c5b75283e..ebbdb7c3c16 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -1677,7 +1677,8 @@ bytes_subscript(PyBytesObject* self, PyObject* item) return PyLong_FromLong((unsigned char)self->ob_sval[i]); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelength, cur, i; + Py_ssize_t start, stop, step, slicelength, i; + size_t cur; char* source_buf; char* result_buf; PyObject* result; diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 9f0fc1cc2c3..dc1d0e5ad0d 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -753,7 +753,8 @@ tuplesubscript(PyTupleObject* self, PyObject* item) return tupleitem(self, i); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelength, cur, i; + Py_ssize_t start, stop, step, slicelength, i; + size_t cur; PyObject* result; PyObject* it; PyObject **src, **dest; diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index eaba5836cb1..0aa5e4ad18d 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -13991,7 +13991,8 @@ unicode_subscript(PyObject* self, PyObject* item) i += PyUnicode_GET_LENGTH(self); return unicode_getitem(self, i); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelength, cur, i; + Py_ssize_t start, stop, step, slicelength, i; + size_t cur; PyObject *result; void *src_data, *dest_data; int src_kind, dest_kind;