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;
