From cf940c701f982d7a38144b31d09dc9613af841b7 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 10 Aug 2010 18:35:01 +0000 Subject: [PATCH] Issue #9530: Fix undefined-behaviour-inducing overflow checks in bytes and bytearray implementations. --- Lib/test/test_bytes.py | 6 +++-- Objects/bytearrayobject.c | 54 +++++++++++++++---------------------- Objects/bytesobject.c | 57 ++++++++++++++++----------------------- 3 files changed, 49 insertions(+), 68 deletions(-) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 16203a59f83..6a402b1ff9a 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -220,8 +220,10 @@ class BaseBytesTest(unittest.TestCase): self.assertRaises(TypeError, lambda: b * 3.14) self.assertRaises(TypeError, lambda: 3.14 * b) # XXX Shouldn't bytes and bytearray agree on what to raise? - self.assertRaises((OverflowError, MemoryError), - lambda: b * sys.maxsize) + with self.assertRaises((OverflowError, MemoryError)): + c = b * sys.maxsize + with self.assertRaises((OverflowError, MemoryError)): + b *= sys.maxsize def test_repeat_1char(self): self.assertEqual(self.type2test(b'x')*100, self.type2test([ord('x')]*100)) diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 33f80d56f55..cdc860fc32f 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -310,9 +310,9 @@ bytearray_repeat(PyByteArrayObject *self, Py_ssize_t count) if (count < 0) count = 0; mysize = Py_SIZE(self); - size = mysize * count; - if (count != 0 && size / count != mysize) + if (count > 0 && mysize > PY_SSIZE_T_MAX / count) return PyErr_NoMemory(); + size = mysize * count; result = (PyByteArrayObject *)PyByteArray_FromStringAndSize(NULL, size); if (result != NULL && size != 0) { if (mysize == 1) @@ -335,9 +335,9 @@ bytearray_irepeat(PyByteArrayObject *self, Py_ssize_t count) if (count < 0) count = 0; mysize = Py_SIZE(self); - size = mysize * count; - if (count != 0 && size / count != mysize) + if (count > 0 && mysize > PY_SSIZE_T_MAX / count) return PyErr_NoMemory(); + size = mysize * count; if (size < self->ob_alloc) { Py_SIZE(self) = size; self->ob_bytes[Py_SIZE(self)] = '\0'; /* Trailing null byte */ @@ -1505,30 +1505,28 @@ replace_interleave(PyByteArrayObject *self, { char *self_s, *result_s; Py_ssize_t self_len, result_len; - Py_ssize_t count, i, product; + Py_ssize_t count, i; PyByteArrayObject *result; self_len = PyByteArray_GET_SIZE(self); - /* 1 at the end plus 1 after every character */ - count = self_len+1; - if (maxcount < count) + /* 1 at the end plus 1 after every character; + count = min(maxcount, self_len + 1) */ + if (maxcount <= self_len) count = maxcount; + else + /* Can't overflow: self_len + 1 <= maxcount <= PY_SSIZE_T_MAX. */ + count = self_len + 1; /* Check for overflow */ /* result_len = count * to_len + self_len; */ - product = count * to_len; - if (product / to_len != count) { - PyErr_SetString(PyExc_OverflowError, - "replace string is too long"); - return NULL; - } - result_len = product + self_len; - if (result_len < 0) { + assert(count > 0); + if (to_len > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replace string is too long"); return NULL; } + result_len = count * to_len + self_len; if (! (result = (PyByteArrayObject *) PyByteArray_FromStringAndSize(NULL, result_len)) ) @@ -1758,7 +1756,7 @@ replace_single_character(PyByteArrayObject *self, char *self_s, *result_s; char *start, *next, *end; Py_ssize_t self_len, result_len; - Py_ssize_t count, product; + Py_ssize_t count; PyByteArrayObject *result; self_s = PyByteArray_AS_STRING(self); @@ -1772,16 +1770,12 @@ replace_single_character(PyByteArrayObject *self, /* use the difference between current and new, hence the "-1" */ /* result_len = self_len + count * (to_len-1) */ - product = count * (to_len-1); - if (product / (to_len-1) != count) { + assert(count > 0); + if (to_len - 1 > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replace bytes is too long"); return NULL; } - result_len = self_len + product; - if (result_len < 0) { - PyErr_SetString(PyExc_OverflowError, "replace bytes is too long"); - return NULL; - } + result_len = self_len + count * (to_len - 1); if ( (result = (PyByteArrayObject *) PyByteArray_FromStringAndSize(NULL, result_len)) == NULL) @@ -1825,7 +1819,7 @@ replace_substring(PyByteArrayObject *self, char *self_s, *result_s; char *start, *next, *end; Py_ssize_t self_len, result_len; - Py_ssize_t count, offset, product; + Py_ssize_t count, offset; PyByteArrayObject *result; self_s = PyByteArray_AS_STRING(self); @@ -1842,16 +1836,12 @@ replace_substring(PyByteArrayObject *self, /* Check for overflow */ /* result_len = self_len + count * (to_len-from_len) */ - product = count * (to_len-from_len); - if (product / (to_len-from_len) != count) { - PyErr_SetString(PyExc_OverflowError, "replace bytes is too long"); - return NULL; - } - result_len = self_len + product; - if (result_len < 0) { + assert(count > 0); + if (to_len - from_len > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replace bytes is too long"); return NULL; } + result_len = self_len + count * (to_len - from_len); if ( (result = (PyByteArrayObject *) PyByteArray_FromStringAndSize(NULL, result_len)) == NULL) diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index eb8036065a1..c0c82f73a66 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -579,13 +579,14 @@ PyBytes_Repr(PyObject *obj, int smartquotes) static const char *hexdigits = "0123456789abcdef"; register PyBytesObject* op = (PyBytesObject*) obj; Py_ssize_t length = Py_SIZE(op); - size_t newsize = 3 + 4 * length; + size_t newsize; PyObject *v; - if (newsize > PY_SSIZE_T_MAX || (newsize-3) / 4 != length) { + if (length > (PY_SSIZE_T_MAX - 3) / 4) { PyErr_SetString(PyExc_OverflowError, "bytes object is too large to make repr"); return NULL; } + newsize = 3 + 4 * length; v = PyUnicode_FromUnicode(NULL, newsize); if (v == NULL) { return NULL; @@ -732,12 +733,12 @@ bytes_repeat(register PyBytesObject *a, register Py_ssize_t n) /* watch out for overflows: the size can overflow int, * and the # of bytes needed can overflow size_t */ - size = Py_SIZE(a) * n; - if (n && size / n != Py_SIZE(a)) { + if (n > 0 && Py_SIZE(a) > PY_SSIZE_T_MAX / n) { PyErr_SetString(PyExc_OverflowError, "repeated bytes are too long"); return NULL; } + size = Py_SIZE(a) * n; if (size == Py_SIZE(a) && PyBytes_CheckExact(a)) { Py_INCREF(a); return (PyObject *)a; @@ -1687,30 +1688,28 @@ replace_interleave(PyBytesObject *self, { char *self_s, *result_s; Py_ssize_t self_len, result_len; - Py_ssize_t count, i, product; + Py_ssize_t count, i; PyBytesObject *result; self_len = PyBytes_GET_SIZE(self); - /* 1 at the end plus 1 after every character */ - count = self_len+1; - if (maxcount < count) + /* 1 at the end plus 1 after every character; + count = min(maxcount, self_len + 1) */ + if (maxcount <= self_len) count = maxcount; + else + /* Can't overflow: self_len + 1 <= maxcount <= PY_SSIZE_T_MAX. */ + count = self_len + 1; /* Check for overflow */ /* result_len = count * to_len + self_len; */ - product = count * to_len; - if (product / to_len != count) { - PyErr_SetString(PyExc_OverflowError, - "replacement bytes are too long"); - return NULL; - } - result_len = product + self_len; - if (result_len < 0) { + assert(count > 0); + if (to_len > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replacement bytes are too long"); return NULL; } + result_len = count * to_len + self_len; if (! (result = (PyBytesObject *) PyBytes_FromStringAndSize(NULL, result_len)) ) @@ -1939,7 +1938,7 @@ replace_single_character(PyBytesObject *self, char *self_s, *result_s; char *start, *next, *end; Py_ssize_t self_len, result_len; - Py_ssize_t count, product; + Py_ssize_t count; PyBytesObject *result; self_s = PyBytes_AS_STRING(self); @@ -1953,18 +1952,13 @@ replace_single_character(PyBytesObject *self, /* use the difference between current and new, hence the "-1" */ /* result_len = self_len + count * (to_len-1) */ - product = count * (to_len-1); - if (product / (to_len-1) != count) { + assert(count > 0); + if (to_len - 1 > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replacement bytes are too long"); return NULL; } - result_len = self_len + product; - if (result_len < 0) { - PyErr_SetString(PyExc_OverflowError, - "replacment bytes are too long"); - return NULL; - } + result_len = self_len + count * (to_len - 1); if ( (result = (PyBytesObject *) PyBytes_FromStringAndSize(NULL, result_len)) == NULL) @@ -2007,7 +2001,7 @@ replace_substring(PyBytesObject *self, char *self_s, *result_s; char *start, *next, *end; Py_ssize_t self_len, result_len; - Py_ssize_t count, offset, product; + Py_ssize_t count, offset; PyBytesObject *result; self_s = PyBytes_AS_STRING(self); @@ -2024,18 +2018,13 @@ replace_substring(PyBytesObject *self, /* Check for overflow */ /* result_len = self_len + count * (to_len-from_len) */ - product = count * (to_len-from_len); - if (product / (to_len-from_len) != count) { - PyErr_SetString(PyExc_OverflowError, - "replacement bytes are too long"); - return NULL; - } - result_len = self_len + product; - if (result_len < 0) { + assert(count > 0); + if (to_len - from_len > (PY_SSIZE_T_MAX - self_len) / count) { PyErr_SetString(PyExc_OverflowError, "replacement bytes are too long"); return NULL; } + result_len = self_len + count * (to_len-from_len); if ( (result = (PyBytesObject *) PyBytes_FromStringAndSize(NULL, result_len)) == NULL)