Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
remicollet committed Dec 28, 2013
1 parent aba76f0 commit 8f4a537
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 23 deletions.
3 changes: 2 additions & 1 deletion NEWS
Expand Up @@ -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)

Expand Down
52 changes: 32 additions & 20 deletions ext/gd/libgd/gd_crop.c
Expand Up @@ -44,44 +44,56 @@ 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);
} else {
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;
}

/**
Expand Down
22 changes: 20 additions & 2 deletions ext/gd/tests/bug66356.phpt
Expand Up @@ -7,16 +7,34 @@ 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
[y] => 12b
[width] => 10
[height] => 10
)
bool(false)
resource(%d) of type (gd)
resource(%d) of type (gd)

0 comments on commit 8f4a537

Please sign in to comment.