Fix signed integer overflow in Imagick::importImagePixels()

imagick_class.c

@@ -2400,6 +2400,8 @@ PHP_METHOD(Imagick, importImagePixels)
 	char *map;
 	zval *pixels;
 	HashTable *array;
+	size_t pixel_count;
+	size_t expected_elements;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "llllsla", &x, &y, &width, &height, &map, &map_len, &storage, &pixels) == FAILURE) {
 		RETURN_THROWS();
@@ -2420,16 +2422,30 @@ PHP_METHOD(Imagick, importImagePixels)
 
 	array = Z_ARRVAL_P(pixels);
 
-	if (zend_hash_num_elements(array) != ((width * height) * map_len)) {
+	if ((size_t) height > ((size_t) -1) / (size_t) width) {
+		php_imagick_throw_exception(IMAGICK_CLASS, "The requested dimensions are too large to process" TSRMLS_CC);
+		RETURN_THROWS();
+	}
+
+	pixel_count = (size_t) width * (size_t) height;
+
+	if (map_len != 0 && (size_t) map_len > ((size_t) -1) / pixel_count) {
+		php_imagick_throw_exception(IMAGICK_CLASS, "The requested dimensions are too large to process" TSRMLS_CC);
+		RETURN_THROWS();
+	}
+
+	expected_elements = pixel_count * (size_t) map_len;
+
+	if (zend_hash_num_elements(array) != expected_elements) {
 		zend_throw_exception_ex(
 			php_imagick_exception_class_entry,
 				0,
 #if PHP_VERSION_ID >= 70000
-				"The map contains incorrect number of elements. Expected %ld, array has %u",
+				"The map contains incorrect number of elements. Expected %zu, array has %u",
 #else
-				"The map contains incorrect number of elements. Expected %ld, array has %d",
+				"The map contains incorrect number of elements. Expected %zu, array has %d",
 #endif
-				(width * height) * map_len,
+				expected_elements,
 				zend_hash_num_elements(array)
 			);
 

package.xml

@@ -408,6 +408,7 @@ This extension requires ImageMagick version 6.5.3-10+ and PHP 5.6.0+.
 				<file name="329_imagick_getImageBlob_empty.phpt" role="test" />
 				<file name="330_Imagick_newImage.phpt" role="test" />
 				<file name="331_Imagick_getImagesBlob_empty.phpt" role="test" />
+				<file name="332_Imagick_importImagePixels_overflow.phpt" role="test" />
 				<file name="bug20636.phpt" role="test" />
 				<file name="bug21229.phpt" role="test" />
 				<file name="bug59378.phpt" role="test" />

tests/332_Imagick_importImagePixels_overflow.phpt

@@ -0,0 +1,51 @@
+--TEST--
+Test Imagick::importImagePixels() rejects dimensions that overflow instead of triggering undefined behaviour
+--SKIPIF--
+<?php
+require_once(dirname(__FILE__) . '/skipif.inc');
+
+if (!method_exists("Imagick", "importImagePixels")) {
+	die("skip Imagick::importImagePixels not available");
+}
+
+if (PHP_INT_SIZE < 8) {
+	die("skip 64-bit platform required");
+}
+?>
+--FILE--
+<?php
+
+$im = new Imagick();
+$im->newImage(1, 1, 'gray');
+
+// width*height overflow
+try {
+	$im->importImagePixels(0, 0, 4294967296, 4294967296, "RGB", Imagick::PIXEL_CHAR, array());
+	echo "no exception thrown\n";
+} catch (ImagickException $e) {
+	echo $e->getMessage() . "\n";
+}
+
+// width*height*map overflow
+try {
+	$im->importImagePixels(0, 0, 4294967295, 4294967295, "RGB", Imagick::PIXEL_CHAR, array());
+	echo "no exception thrown\n";
+} catch (ImagickException $e) {
+	echo $e->getMessage() . "\n";
+}
+
+// sane but incorrect count
+try {
+	$im->importImagePixels(0, 0, 2, 2, "RGB", Imagick::PIXEL_CHAR, array(0, 0, 0));
+	echo "no exception thrown\n";
+} catch (ImagickException $e) {
+	echo $e->getMessage() . "\n";
+}
+
+echo "Ok\n";
+?>
+--EXPECTF--
+The requested dimensions are too large to process
+The requested dimensions are too large to process
+The map contains incorrect number of elements. Expected 12, array has 3
+Ok