Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop())

Initial fix was PHP stuff
This one is libgd fix.

- filter invalid crop size
- dont try to copy on invalid position
- fix crop size when out of src image
- fix possible NULL deref
- fix possible integer overfloow

diff --git a/NEWS b/NEWS
index 86eeb7cea9fef34108eb023e82e5b38875db57e6..aed16cce65f17eb2862c94045f5c15ac018a9ab3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -29,7 +29,8 @@ PHP                                                                        NEWS
   . Fixed bug #66229 (128.0.0.0/16 isn't reserved any longer). (Adam)
 
 - GD:
-  . Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop()). (Laruence)
+  . Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop()).
+    (Laruence, Remi)
   . Fixed bug #64405 (Use freetype-config for determining freetype2 dir(s)).
     (Adam)
     

diff --git a/ext/gd/libgd/gd_crop.c b/ext/gd/libgd/gd_crop.c
index f0b888a4f1c69ed4b6dcc563153ea8ec7152b64d..90a99a650a811721a176fb7c1e318350f80e9c68 100644 (file)
--- a/ext/gd/libgd/gd_crop.c
+++ b/ext/gd/libgd/gd_crop.c
@@ -44,6 +44,12 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop)
 {
        gdImagePtr dst;
 
+       /* check size */
+       if (crop->width<=0 || crop->height<=0) {
+               return NULL;
+       }
+
+       /* allocate the requested size (could be only partially filled) */
        if (src->trueColor) {
                dst = gdImageCreateTrueColor(crop->width, crop->height);
                gdImageSaveAlpha(dst, 1);
@@ -51,37 +57,43 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop)
                dst = gdImageCreate(crop->width, crop->height);
                gdImagePaletteCopy(dst, src);
        }
+       if (dst == NULL) {
+               return NULL;
+       }
        dst->transparent = src->transparent;
 
-       if (src->sx < (crop->x + crop->width -1)) {
-               crop->width = src->sx - crop->x + 1;
+       /* check position in the src image */
+       if (crop->x < 0 || crop->x>=src->sx || crop->y<0 || crop->y>=src->sy) {
+               return dst;
+       }
+
+       /* reduce size if needed */
+       if ((src->sx - crop->width) < crop->x) {
+               crop->width = src->sx - crop->x;
        }
-       if (src->sy < (crop->y + crop->height -1)) {
-               crop->height = src->sy - crop->y + 1;
+       if ((src->sy - crop->height) < crop->y) {
+               crop->height = src->sy - crop->y;
        }
+
 #if 0
 printf("rect->x: %i\nrect->y: %i\nrect->width: %i\nrect->height: %i\n", crop->x, crop->y, crop->width, crop->height);
 #endif
-       if (dst == NULL) {
-               return NULL;
+       int y = crop->y;
+       if (src->trueColor) {
+               unsigned int dst_y = 0;
+               while (y < (crop->y + (crop->height - 1))) {
+                       /* TODO: replace 4 w/byte per channel||pitch once available */
+                       memcpy(dst->tpixels[dst_y++], src->tpixels[y++] + crop->x, crop->width * 4);
+               }
        } else {
-               int y = crop->y;
-               if (src->trueColor) {
-                       unsigned int dst_y = 0;
-                       while (y < (crop->y + (crop->height - 1))) {
-                               /* TODO: replace 4 w/byte per channel||pitch once available */
-                               memcpy(dst->tpixels[dst_y++], src->tpixels[y++] + crop->x, crop->width * 4);
-                       }
-               } else {
-                       int x;
-                       for (y = crop->y; y < (crop->y + (crop->height - 1)); y++) {
-                               for (x = crop->x; x < (crop->x + (crop->width - 1)); x++) {
-                                       dst->pixels[y - crop->y][x - crop->x] = src->pixels[y][x];
-                               }
+               int x;
+               for (y = crop->y; y < (crop->y + (crop->height - 1)); y++) {
+                       for (x = crop->x; x < (crop->x + (crop->width - 1)); x++) {
+                               dst->pixels[y - crop->y][x - crop->x] = src->pixels[y][x];
                        }
                }
-               return dst;
        }
+       return dst;
 }
 
 /**

diff --git a/ext/gd/tests/bug66356.phpt b/ext/gd/tests/bug66356.phpt
index f881494716402338847ab4ff23619f4f633fc384..2da91d61a9f3e2abe2b6c3973db5c5077f7b89fe 100644 (file)
--- a/ext/gd/tests/bug66356.phpt
+++ b/ext/gd/tests/bug66356.phpt
@@ -7,12 +7,27 @@ Bug #66356 (Heap Overflow Vulnerability in imagecrop())
 --FILE--
 <?php
 $img = imagecreatetruecolor(10, 10);
-$img = imagecrop($img, array("x" => "a", "y" => 0, "width" => 10, "height" => 10));
+
+// POC #1
+var_dump(imagecrop($img, array("x" => "a", "y" => 0, "width" => 10, "height" => 10)));
+
 $arr = array("x" => "a", "y" => "12b", "width" => 10, "height" => 10);
-$img = imagecrop($img, $arr);
+var_dump(imagecrop($img, $arr));
 print_r($arr);
+
+// POC #2
+var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => -1, "height" => 10)));
+
+// POC #3
+var_dump(imagecrop($img, array("x" => -20, "y" => -20, "width" => 10, "height" => 10)));
+
+// POC #4
+var_dump(imagecrop($img, array("x" => 0x7fffff00, "y" => 0, "width" => 10, "height" => 10)));
+
 ?>
 --EXPECTF--
+resource(%d) of type (gd)
+resource(%d) of type (gd)
 Array
 (
     [x] => a
@@ -20,3 +35,6 @@ Array
     [width] => 10
     [height] => 10
 )
+bool(false)
+resource(%d) of type (gd)
+resource(%d) of type (gd)
\ No newline at end of file