From: Christoph M. Becker <cmbecker69@gmx.de>
Date: Sun, 25 Nov 2018 14:41:27 +0000 (+0100)
Subject: Fix #77198: auto cropping has insufficient precision
X-Git-Tag: php-7.2.14RC1~42
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b47b8886dd17d080c74c401f7893ba9f4ccb83d3;p=php

Fix #77198: auto cropping has insufficient precision

We apply the upstream patch[1], and also fix the erroneous bailout at
the end of `gdImageAutoCrop()`, since `crop.x` and `crop.y` may very
well be zero.

[1] <https://github.com/libgd/libgd/commit/bda85aaeeb1d7467d92c892ba4c30eaa53d7a6d4>
---

diff --git a/NEWS b/NEWS
index cdd38f5e58..238c30c75a 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,7 @@ PHP                                                                        NEWS
 
 - GD:
   . Fixed bug #77195 (Incorrect error handling of imagecreatefromjpeg()). (cmb)
+  . Fixed bug #77198 (auto cropping has insufficient precision). (cmb)
 
 - Sockets:
   . Fixed bug #77136 (Unsupported IPV6_RECVPKTINFO constants on macOS).
diff --git a/ext/gd/libgd/gd_crop.c b/ext/gd/libgd/gd_crop.c
index 58b630317d..0b918c74c8 100644
--- a/ext/gd/libgd/gd_crop.c
+++ b/ext/gd/libgd/gd_crop.c
@@ -163,30 +163,24 @@ gdImagePtr gdImageCropAuto(gdImagePtr im, const unsigned int mode)
 		}
 	}
 
-	/* Nothing to do > bye
-	 * Duplicate the image?
-	 */
-	if (y == height - 1) {
+	/* Whole image would be cropped > bye */
+	if (match) {
 		return NULL;
 	}
 
-	crop.y = y -1;
+	crop.y = y - 1;
+
 	match = 1;
 	for (y = height - 1; match && y >= 0; y--) {
 		for (x = 0; match && x < width; x++) {
 			match = (color == gdImageGetPixel(im, x,y));
 		}
 	}
-
-	if (y == 0) {
-		crop.height = height - crop.y + 1;
-	} else {
-		crop.height = y - crop.y + 2;
-	}
+	crop.height = y - crop.y + 2;
 
 	match = 1;
 	for (x = 0; match && x < width; x++) {
-		for (y = 0; match && y < crop.y + crop.height - 1; y++) {
+		for (y = 0; match && y < crop.y + crop.height; y++) {
 			match = (color == gdImageGetPixel(im, x,y));
 		}
 	}
@@ -194,12 +188,13 @@ gdImagePtr gdImageCropAuto(gdImagePtr im, const unsigned int mode)
 
 	match = 1;
 	for (x = width - 1; match && x >= 0; x--) {
-		for (y = 0; match &&  y < crop.y + crop.height - 1; y++) {
+		for (y = 0; match &&  y < crop.y + crop.height; y++) {
 			match = (color == gdImageGetPixel(im, x,y));
 		}
 	}
 	crop.width = x - crop.x + 2;
-	if (crop.x <= 0 || crop.y <= 0 || crop.width <= 0 || crop.height <= 0) {
+
+	if (crop.x < 0 || crop.y < 0 || crop.width <= 0 || crop.height <= 0) {
 		return NULL;
 	}
 	return gdImageCrop(im, &crop);
@@ -258,31 +253,24 @@ gdImagePtr gdImageCropThreshold(gdImagePtr im, const unsigned int color, const f
 		}
 	}
 
-	/* Pierre
-	 * Nothing to do > bye
-	 * Duplicate the image?
-	 */
-	if (y == height - 1) {
+	/* Whole image would be cropped > bye */
+	if (match) {
 		return NULL;
 	}
 
-	crop.y = y -1;
+	crop.y = y - 1;
+
 	match = 1;
 	for (y = height - 1; match && y >= 0; y--) {
 		for (x = 0; match && x < width; x++) {
 			match = (gdColorMatch(im, color, gdImageGetPixel(im, x, y), threshold)) > 0;
 		}
 	}
-
-	if (y == 0) {
-		crop.height = height - crop.y + 1;
-	} else {
-		crop.height = y - crop.y + 2;
-	}
+	crop.height = y - crop.y + 2;
 
 	match = 1;
 	for (x = 0; match && x < width; x++) {
-		for (y = 0; match && y < crop.y + crop.height - 1; y++) {
+		for (y = 0; match && y < crop.y + crop.height; y++) {
 			match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0;
 		}
 	}
@@ -290,7 +278,7 @@ gdImagePtr gdImageCropThreshold(gdImagePtr im, const unsigned int color, const f
 
 	match = 1;
 	for (x = width - 1; match && x >= 0; x--) {
-		for (y = 0; match &&  y < crop.y + crop.height - 1; y++) {
+		for (y = 0; match &&  y < crop.y + crop.height; y++) {
 			match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0;
 		}
 	}
diff --git a/ext/gd/tests/bug77198_auto.phpt b/ext/gd/tests/bug77198_auto.phpt
new file mode 100644
index 0000000000..d06f2be663
--- /dev/null
+++ b/ext/gd/tests/bug77198_auto.phpt
@@ -0,0 +1,50 @@
+--TEST--
+Bug #77198 (auto cropping has insufficient precision)
+--SKIPIF--
+<?php
+if (!extension_loaded('gd')) die('skip gd extension not available');
+if (!GD_BUNDLED) die('upstream bugfix has not been released');
+?>
+--FILE--
+<?php
+
+function createWhiteImageWithBlackPixelAt($x, $y)
+{
+    $im = imagecreatetruecolor(8, 8);
+    imagefilledrectangle($im, 0, 0, 7, 7, 0xffffff);
+    imagesetpixel($im, $x, $y, 0x000000);
+    return $im;
+}
+
+for ($y = 0; $y < 8; $y++) {
+    for ($x = 0; $x < 8; $x++) {
+        if (($x == 0 && ($y == 0 || $y == 7)) || ($x == 7 && ($y == 0 || $y == 7))) {
+            continue; // skip the corners
+        }
+        $orig = createWhiteImageWithBlackPixelAt($x, $y);
+        $cropped = imagecropauto($orig, IMG_CROP_SIDES);
+        if (!$cropped) {
+            printf("Pixel at %d, %d: unexpected NULL crop\n", $x, $y);
+        } else {
+            $width = imagesx($cropped);
+            if ($width !== 1) {
+                printf("Pixel at %d, %d: unexpected width (%d)\n", $x, $y, $width);
+            }
+            $height = imagesy($cropped);
+            if ($height !== 1) {
+                printf("Pixel at %d, %d: unexpected height (%d)\n", $x, $y, $height);
+            }
+            $color = imagecolorat($cropped, 0, 0);
+            if ($color !== 0x000000) {
+                printf("Pixel at %d, %d: unexpected color (%d)\n", $x, $y, $color);
+            }
+            imagedestroy($cropped);
+        }
+        imagedestroy($orig);
+    }
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/ext/gd/tests/bug77198_threshold.phpt b/ext/gd/tests/bug77198_threshold.phpt
new file mode 100644
index 0000000000..fd03660c84
--- /dev/null
+++ b/ext/gd/tests/bug77198_threshold.phpt
@@ -0,0 +1,47 @@
+--TEST--
+Bug #77198 (threshold cropping has insufficient precision)
+--SKIPIF--
+<?php
+if (!extension_loaded('gd')) die('skip gd extension not available');
+if (!GD_BUNDLED) die('upstream bugfix has not been released');
+?>
+--FILE--
+<?php
+
+function createWhiteImageWithBlackPixelAt($x, $y)
+{
+    $im = imagecreatetruecolor(8, 8);
+    imagefilledrectangle($im, 0, 0, 7, 7, 0xffffff);
+    imagesetpixel($im, $x, $y, 0x000000);
+    return $im;
+}
+
+for ($y = 0; $y < 8; $y++) {
+    for ($x = 0; $x < 8; $x++) {
+        $orig = createWhiteImageWithBlackPixelAt($x, $y);
+        $cropped = imagecropauto($orig, IMG_CROP_THRESHOLD, 1, 0xffffff);
+        if (!$cropped) {
+            printf("Pixel at %d, %d: unexpected NULL crop\n", $x, $y);
+        } else {
+            $width = imagesx($cropped);
+            if ($width !== 1) {
+                printf("Pixel at %d, %d: unexpected width (%d)\n", $x, $y, $width);
+            }
+            $height = imagesy($cropped);
+            if ($height !== 1) {
+                printf("Pixel at %d, %d: unexpected height (%d)\n", $x, $y, $height);
+            }
+            $color = imagecolorat($cropped, 0, 0);
+            if ($color !== 0x000000) {
+                printf("Pixel at %d, %d: unexpected color (%d)\n", $x, $y, $color);
+            }
+            imagedestroy($cropped);
+        }
+        imagedestroy($orig);
+    }
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===