From 4a3acca7c1f85946ac7526e303956dc03acd91fe Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 11 Jun 2010 20:08:36 +0000 Subject: Merged revisions 81897-81898,81902 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k ........ r81897 | mark.dickinson | 2010-06-11 17:56:34 +0100 (Fri, 11 Jun 2010) | 1 line Avoid possible undefined behaviour from signed overflow. ........ r81898 | mark.dickinson | 2010-06-11 20:05:08 +0100 (Fri, 11 Jun 2010) | 1 line Fix an incorrect return type. ........ r81902 | mark.dickinson | 2010-06-11 20:50:30 +0100 (Fri, 11 Jun 2010) | 1 line Fix more undefined-behaviour inducing overflow checks in struct module. ........ --- Lib/test/test_struct.py | 7 +++++++ Modules/_struct.c | 51 ++++++++++++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 684dc6c..6032895 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -511,6 +511,13 @@ class StructTest(unittest.TestCase): for c in [b'\x01', b'\x7f', b'\xff', b'\x0f', b'\xf0']: self.assertTrue(struct.unpack('>?', c)[0]) + def test_count_overflow(self): + hugecount = '{}b'.format(sys.maxsize+1) + self.assertRaises(struct.error, struct.calcsize, hugecount) + + hugecount2 = '{}b{}H'.format(sys.maxsize//2, sys.maxsize//2) + self.assertRaises(struct.error, struct.calcsize, hugecount2) + if IS32BIT: def test_crasher(self): self.assertRaises(MemoryError, struct.pack, "357913941b", "a") diff --git a/Modules/_struct.c b/Modules/_struct.c index 3f01c25..c91e2d1 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1132,16 +1132,19 @@ getentry(int c, const formatdef *f) } -/* Align a size according to a format code */ +/* Align a size according to a format code. Return -1 on overflow. */ -static int +static Py_ssize_t align(Py_ssize_t size, char c, const formatdef *e) { + Py_ssize_t extra; + if (e->format == c) { - if (e->alignment) { - size = ((size + e->alignment - 1) - / e->alignment) - * e->alignment; + if (e->alignment && size > 0) { + extra = (e->alignment - 1) - (size - 1) % (e->alignment); + if (extra > PY_SSIZE_T_MAX - size) + return -1; + size += extra; } } return size; @@ -1160,7 +1163,7 @@ prepare_s(PyStructObject *self) const char *s; const char *fmt; char c; - Py_ssize_t size, len, num, itemsize, x; + Py_ssize_t size, len, num, itemsize; fmt = PyBytes_AS_STRING(self->s_format); @@ -1175,14 +1178,13 @@ prepare_s(PyStructObject *self) if ('0' <= c && c <= '9') { num = c - '0'; while ('0' <= (c = *s++) && c <= '9') { - x = num*10 + (c - '0'); - if (x/10 != num) { - PyErr_SetString( - StructError, - "overflow in item count"); - return -1; - } - num = x; + /* overflow-safe version of + if (num*10 + (c - '0') > PY_SSIZE_T_MAX) { ... } */ + if (num >= PY_SSIZE_T_MAX / 10 && ( + num > PY_SSIZE_T_MAX / 10 || + (c - '0') > PY_SSIZE_T_MAX % 10)) + goto overflow; + num = num*10 + (c - '0'); } if (c == '\0') break; @@ -1203,13 +1205,13 @@ prepare_s(PyStructObject *self) itemsize = e->size; size = align(size, c, e); - x = num * itemsize; - size += x; - if (x/itemsize != num || size < 0) { - PyErr_SetString(StructError, - "total struct size too long"); - return -1; - } + if (size == -1) + goto overflow; + + /* if (size + num * itemsize > PY_SSIZE_T_MAX) { ... } */ + if (num > (PY_SSIZE_T_MAX - size) / itemsize) + goto overflow; + size += num * itemsize; } /* check for overflow */ @@ -1268,6 +1270,11 @@ prepare_s(PyStructObject *self) codes->size = 0; return 0; + + overflow: + PyErr_SetString(StructError, + "total struct size too long"); + return -1; } static PyObject * -- cgit v0.12