Author: benjamin.peterson Date: Tue Sep 30 03:46:48 2008 New Revision: 66690 Log: backport r66689: imageop could segfault due to poor argument validation Modified: python/branches/release25-maint/Lib/test/test_imageop.py python/branches/release25-maint/Misc/NEWS python/branches/release25-maint/Modules/imageop.c Modified: python/branches/release25-maint/Lib/test/test_imageop.py ============================================================================== --- python/branches/release25-maint/Lib/test/test_imageop.py (original) +++ python/branches/release25-maint/Lib/test/test_imageop.py Tue Sep 30 03:46:48 2008 @@ -5,9 +5,9 @@ Roger E. Masse """ -from test.test_support import verbose, unlink +from test.test_support import verbose, unlink, run_unittest -import imageop, uu, os +import imageop, uu, os, unittest import warnings warnings.filterwarnings("ignore", @@ -15,7 +15,66 @@ DeprecationWarning, ".*test_imageop") -def main(use_rgbimg=1): +SIZES = (1, 2, 3, 4) +_VALUES = (1, 2, 2**10, 2**15-1, 2**15, 2**15+1, 2**31-2, 2**31-1) +VALUES = tuple( -x for x in reversed(_VALUES) ) + (0,) + _VALUES +AAAAA = "A" * 1024 + + +class InputValidationTests(unittest.TestCase): + + def _check(self, name, size=None, *extra): + func = getattr(imageop, name) + for height in VALUES: + for width in VALUES: + strlen = abs(width * height) + if size: + strlen *= size + if strlen < 1024: + data = "A" * strlen + else: + data = AAAAA + if size: + arguments = (data, size, width, height) + extra + else: + arguments = (data, width, height) + extra + try: + func(*arguments) + except (ValueError, imageop.error): + pass + + def check_size(self, name, *extra): + for size in SIZES: + self._check(name, size, *extra) + + def check(self, name, *extra): + self._check(name, None, *extra) + + def test_input_validation(self): + self.check_size("crop", 0, 0, 0, 0) + self.check_size("scale", 1, 0) + self.check_size("scale", -1, -1) + self.check_size("tovideo") + self.check("grey2mono", 128) + self.check("grey2grey4") + self.check("grey2grey2") + self.check("dither2mono") + self.check("dither2grey2") + self.check("mono2grey", 0, 0) + self.check("grey22grey") + self.check("rgb2rgb8") # nlen*4 == len + self.check("rgb82rgb") + self.check("rgb2grey") + self.check("grey2rgb") + + +def test_main(use_rgbimg=True): + run_unittest(InputValidationTests) + + try: + import imgfile + except ImportError: + return # Create binary test files uu.decode(get_qualified_path('testrgb'+os.extsep+'uue'), 'test'+os.extsep+'rgb') @@ -171,7 +230,3 @@ if os.path.exists(fullname): return fullname return name - -# rgbimg (unlike imgfile) is portable to platforms other than SGI. -# So we prefer to use it. -main(use_rgbimg=1) Modified: python/branches/release25-maint/Misc/NEWS ============================================================================== --- python/branches/release25-maint/Misc/NEWS (original) +++ python/branches/release25-maint/Misc/NEWS Tue Sep 30 03:46:48 2008 @@ -190,6 +190,9 @@ Extension Modules ----------------- +- Security Issue #2: imageop did not validate arguments correctly and could + segfault as a result. + - Issue 3886: [CVE-2008-2316] Possible integer overflow in the _hashopenssl module was closed. Modified: python/branches/release25-maint/Modules/imageop.c ============================================================================== --- python/branches/release25-maint/Modules/imageop.c (original) +++ python/branches/release25-maint/Modules/imageop.c Tue Sep 30 03:46:48 2008 @@ -26,6 +26,46 @@ static PyObject *ImageopError; static PyObject *ImageopDict; +/** + * Check a coordonnate, make sure that (0 < value). + * Return 0 on error. + */ +static int +check_coordonnate(int value, const char* name) +{ + if ( 0 < value) + return 1; + PyErr_Format(PyExc_ValueError, "%s value is negative or nul", name); + return 0; +} + +/** + * Check integer overflow to make sure that product == x*y*size. + * Return 0 on error. + */ +static int +check_multiply_size(int product, int x, const char* xname, int y, const char* yname, int size) +{ + if ( !check_coordonnate(x, xname) ) + return 0; + if ( !check_coordonnate(y, yname) ) + return 0; + if ( size == (product / y) / x ) + return 1; + PyErr_SetString(ImageopError, "String has incorrect length"); + return 0; +} + +/** + * Check integer overflow to make sure that product == x*y. + * Return 0 on error. + */ +static int +check_multiply(int product, int x, int y) +{ + return check_multiply_size(product, x, "x", y, "y", 1); +} + /* If this function returns true (the default if anything goes wrong), we're behaving in a backward-compatible way with respect to how multi-byte pixels are stored in the strings. The code in this module was originally written @@ -85,26 +125,21 @@ if ( !PyArg_ParseTuple(args, "s#iiiiiii", &cp, &len, &size, &x, &y, &newx1, &newy1, &newx2, &newy2) ) return 0; - + if ( size != 1 && size != 2 && size != 4 ) { PyErr_SetString(ImageopError, "Size should be 1, 2 or 4"); return 0; } - if (( len != size*x*y ) || - ( size != ((len / x) / y) )) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(len, x, "x", y, "y", size) ) return 0; - } + xstep = (newx1 < newx2)? 1 : -1; ystep = (newy1 < newy2)? 1 : -1; - + nlen = (abs(newx2-newx1)+1)*(abs(newy2-newy1)+1)*size; - if ( size != ((nlen / (abs(newx2-newx1)+1)) / (abs(newy2-newy1)+1)) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(nlen, abs(newx2-newx1)+1, "abs(newx2-newx1)+1", abs(newy2-newy1)+1, "abs(newy2-newy1)+1", size) ) return 0; - } - rv = PyString_FromStringAndSize(NULL, - (abs(newx2-newx1)+1)*(abs(newy2-newy1)+1)*size); + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; ncp = (char *)PyString_AsString(rv); @@ -131,7 +166,7 @@ } return rv; } - + static PyObject * imageop_scale(PyObject *self, PyObject *args) { @@ -146,22 +181,17 @@ if ( !PyArg_ParseTuple(args, "s#iiiii", &cp, &len, &size, &x, &y, &newx, &newy) ) return 0; - + if ( size != 1 && size != 2 && size != 4 ) { PyErr_SetString(ImageopError, "Size should be 1, 2 or 4"); return 0; } - if ( ( len != size*x*y ) || - ( size != ((len / x) / y) ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(len, x, "x", y, "y", size) ) return 0; - } nlen = newx*newy*size; - if ( size != ((nlen / newx) / newy) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(nlen, newx, "newx", newy, "newy", size) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -193,8 +223,8 @@ unsigned char *cp, *ncp; int width; PyObject *rv; - - + + if ( !PyArg_ParseTuple(args, "s#iii", &cp, &len, &width, &maxx, &maxy) ) return 0; @@ -202,12 +232,9 @@ PyErr_SetString(ImageopError, "Size should be 1 or 4"); return 0; } - if ( ( maxx*maxy*width != len ) || - ( maxx != ((len / maxy) / width) ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(len, maxx, "max", maxy, "maxy", width) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, len); if ( rv == 0 ) return 0; @@ -248,17 +275,14 @@ unsigned char ovalue; PyObject *rv; int i, bit; - - + + if ( !PyArg_ParseTuple(args, "s#iii", &cp, &len, &x, &y, &tres) ) return 0; - if ( ( x*y != len ) || - ( x != len / y ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, (len+7)/8); if ( rv == 0 ) return 0; @@ -290,17 +314,14 @@ PyObject *rv; int i; int pos; - - + + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - if ( ( x*y != len ) || - ( x != len / y ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, (len+1)/2); if ( rv == 0 ) return 0; @@ -330,17 +351,14 @@ PyObject *rv; int i; int pos; - - + + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - if ( ( x*y != len ) || - ( x != len / y ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, (len+3)/4); if ( rv == 0 ) return 0; @@ -369,17 +387,14 @@ unsigned char ovalue; PyObject *rv; int i, bit; - - + + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - if ( ( x*y != len ) || - ( x != len / y ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, (len+7)/8); if ( rv == 0 ) return 0; @@ -416,17 +431,14 @@ int i; int pos; int sum = 0, nvalue; - - + + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - if ( ( x*y != len ) || - ( x != len / y ) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, (len+3)/4); if ( rv == 0 ) return 0; @@ -457,20 +469,18 @@ unsigned char *cp, *ncp; PyObject *rv; int i, bit; - + if ( !PyArg_ParseTuple(args, "s#iiii", &cp, &len, &x, &y, &v0, &v1) ) return 0; nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(nlen, x, y) ) return 0; - } if ( (nlen+7)/8 != len ) { PyErr_SetString(ImageopError, "String has incorrect length"); return 0; } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -498,20 +508,19 @@ unsigned char *cp, *ncp; PyObject *rv; int i, pos, value = 0, nvalue; - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(nlen, x, y) ) { return 0; } if ( (nlen+3)/4 != len ) { PyErr_SetString(ImageopError, "String has incorrect length"); return 0; } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -538,20 +547,18 @@ unsigned char *cp, *ncp; PyObject *rv; int i, pos, value = 0, nvalue; - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(nlen, x, y) ) return 0; - } if ( (nlen+1)/2 != len ) { PyErr_SetString(ImageopError, "String has incorrect length"); return 0; } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -579,20 +586,16 @@ PyObject *rv; int i, r, g, b; int backward_compatible = imageop_backward_compatible(); - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(len*4, x, "x", y, "y", 4) ) return 0; - } - if ( nlen*4 != len ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + nlen = x*y; + if ( !check_multiply(nlen, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -627,31 +630,22 @@ int i, r, g, b; unsigned char value; int backward_compatible = imageop_backward_compatible(); - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); - return 0; - } - if ( nlen != len ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - - if ( nlen / x != y || nlen > INT_MAX / 4) { - PyErr_SetString(ImageopError, "Image is too large"); + nlen = x*y*4; + if ( !check_multiply_size(nlen, x, "x", y, "y", 4) ) return 0; - } - - rv = PyString_FromStringAndSize(NULL, nlen*4); + + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; ncp = (unsigned char *)PyString_AsString(rv); - for ( i=0; i < nlen; i++ ) { + for ( i=0; i < len; i++ ) { /* Bits in source: RRRBBGGG ** Red and Green are multiplied by 36.5, Blue by 85 */ @@ -686,20 +680,16 @@ int i, r, g, b; int nvalue; int backward_compatible = imageop_backward_compatible(); - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply_size(len, x, "x", y, "y", 4) ) return 0; - } - if ( nlen*4 != len ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + nlen = x*y; + if ( !check_multiply(nlen, x, y) ) return 0; - } - + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; @@ -735,31 +725,22 @@ int i; unsigned char value; int backward_compatible = imageop_backward_compatible(); - + if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) ) return 0; - nlen = x*y; - if ( x != (nlen / y) ) { - PyErr_SetString(ImageopError, "String has incorrect length"); - return 0; - } - if ( nlen != len ) { - PyErr_SetString(ImageopError, "String has incorrect length"); + if ( !check_multiply(len, x, y) ) return 0; - } - - if ( nlen / x != y || nlen > INT_MAX / 4) { - PyErr_SetString(ImageopError, "Image is too large"); + nlen = x*y*4; + if ( !check_multiply_size(nlen, x, "x", y, "y", 4) ) return 0; - } - - rv = PyString_FromStringAndSize(NULL, nlen*4); + + rv = PyString_FromStringAndSize(NULL, nlen); if ( rv == 0 ) return 0; ncp = (unsigned char *)PyString_AsString(rv); - for ( i=0; i < nlen; i++ ) { + for ( i=0; i < len; i++ ) { value = *cp++; if (backward_compatible) { * (Py_UInt32 *) ncp = (Py_UInt32) value | ((Py_UInt32) value << 8 ) | ((Py_UInt32) value << 16); @@ -774,39 +755,6 @@ return rv; } -/* -static object * -imageop_mul(object *self, object *args) -{ - char *cp, *ncp; - int len, size, x, y; - object *rv; - int i; - - if ( !getargs(args, "(s#iii)", &cp, &len, &size, &x, &y) ) - return 0; - - if ( size != 1 && size != 4 ) { - err_setstr(ImageopError, "Size should be 1 or 4"); - return 0; - } - if ( len != size*x*y ) { - err_setstr(ImageopError, "String has incorrect length"); - return 0; - } - - rv = newsizedstringobject(NULL, XXXX); - if ( rv == 0 ) - return 0; - ncp = (char *)getstringvalue(rv); - - - for ( i=0; i < len; i += size ) { - } - return rv; -} -*/ - static PyMethodDef imageop_methods[] = { { "crop", imageop_crop, METH_VARARGS }, { "scale", imageop_scale, METH_VARARGS }, @@ -831,6 +779,7 @@ initimageop(void) { PyObject *m; + m = Py_InitModule("imageop", imageop_methods); if (m == NULL) return;