Re: com php-src: Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop()): NEWS ext/gd/libgd/gd_crop.c ext/gd/tests/bug66356.phpt

Hey: Thanks for taking care of this :) thanks On Sat, Dec 28, 2013 at 9:22 PM, Remi Collet <remi@php.net> wrote: > Commit: 8f4a5373bb71590352fd934028d6dde5bc18530b > Author: Remi Collet <remi@php.net> Sat, 28 Dec 2013 14:22:13 +0100 > Parents: aba76f09fa3adca5208b877ed75b2c2ef8f4498f > Branches: PHP-5.5 PHP-5.6 master > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=8f4a5373bb71590352fd934028d6dde5bc18530b > > Log: > 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 > > Bugs: > https://bugs.php.net/66356 > > Changed paths: > M NEWS > M ext/gd/libgd/gd_crop.c > M ext/gd/tests/bug66356.phpt > > > Diff: > diff --git a/NEWS b/NEWS > index 86eeb7c..aed16cc 100644 > --- 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 f0b888a..90a99a6 100644 > --- 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 f881494..2da91d6 100644 > --- 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 > > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > -- Laruence Xinchen Hui http://www.laruence.com/