[Python-checkins] r67200 - in python/branches/release24-maint: Lib/test/test_imageop.py Misc/NEWS Modules/imageop.c

matthias.klose python-checkins at python.org
Wed Nov 12 08:29:24 CET 2008


Author: matthias.klose
Date: Wed Nov 12 08:29:23 2008
New Revision: 67200

Log:
- Security Issue #2: imageop did not validate arguments correctly and could
  segfault as a result. CVE-2008-4864.

backport r66689


Modified:
   python/branches/release24-maint/Lib/test/test_imageop.py
   python/branches/release24-maint/Misc/NEWS
   python/branches/release24-maint/Modules/imageop.c

Modified: python/branches/release24-maint/Lib/test/test_imageop.py
==============================================================================
--- python/branches/release24-maint/Lib/test/test_imageop.py	(original)
+++ python/branches/release24-maint/Lib/test/test_imageop.py	Wed Nov 12 08:29:23 2008
@@ -5,11 +5,70 @@
    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
 
-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')
@@ -165,7 +224,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/release24-maint/Misc/NEWS
==============================================================================
--- python/branches/release24-maint/Misc/NEWS	(original)
+++ python/branches/release24-maint/Misc/NEWS	Wed Nov 12 08:29:23 2008
@@ -35,6 +35,9 @@
   less than zero will now raise a SystemError and return NULL to indicate a
   bug in the calling C code. CVE-2008-1887.
 
+- Security Issue #2: imageop did not validate arguments correctly and could
+  segfault as a result. CVE-2008-4864.
+
 Extension Modules
 -----------------
 

Modified: python/branches/release24-maint/Modules/imageop.c
==============================================================================
--- python/branches/release24-maint/Modules/imageop.c	(original)
+++ python/branches/release24-maint/Modules/imageop.c	Wed Nov 12 08:29:23 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;


More information about the Python-checkins mailing list