summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRemi Collet <remi@php.net>2014-03-05 10:40:36 +0100
committerJulien Pauli <jpauli@php.net>2014-03-05 10:53:57 +0100
commit0880851f004ba8ad793b9fb68e3ee6755245a0ba (patch)
tree381d131ca2bdb7be0b755504dcbbe46c0d41cb17
parent53c6b594f5a0dc1650acfef37d6282d15fea593f (diff)
downloadphp-git-0880851f004ba8ad793b9fb68e3ee6755245a0ba.tar.gz
Fixed Bug #66815 imagecrop(): insufficient fix for NULL defer CVE-2013-7327
This amends commit 8f4a537, which aimed to correct NULL dereference because of missing check of gdImageCreateTrueColor() / gdImageCreate() return value. That commit checks for negative crop rectangle width and height, but gdImageCreate*() can also return NULL when width * height overflows. Hence NULL deref is still possible, as gdImageSaveAlpha() and gdImagePaletteCopy() is called before dst == NULL check. This moves NULL check to happen right after gdImageCreate*(). It also removes width and height check before gdImageCreate*(), as the same check is done by image create functions (with an extra warning). From thoger redhat com
-rw-r--r--ext/gd/libgd/gd_crop.c14
-rw-r--r--ext/gd/tests/bug66356.phpt11
2 files changed, 16 insertions, 9 deletions
diff --git a/ext/gd/libgd/gd_crop.c b/ext/gd/libgd/gd_crop.c
index bba425d0e3..84edb5d1f7 100644
--- a/ext/gd/libgd/gd_crop.c
+++ b/ext/gd/libgd/gd_crop.c
@@ -45,22 +45,20 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop)
gdImagePtr dst;
int y;
- /* 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);
+ if (dst == NULL) {
+ return NULL;
+ }
gdImageSaveAlpha(dst, 1);
} else {
dst = gdImageCreate(crop->width, crop->height);
+ if (dst == NULL) {
+ return NULL;
+ }
gdImagePaletteCopy(dst, src);
}
- if (dst == NULL) {
- return NULL;
- }
dst->transparent = src->transparent;
/* check position in the src image */
diff --git a/ext/gd/tests/bug66356.phpt b/ext/gd/tests/bug66356.phpt
index 2da91d61a9..583d74942c 100644
--- a/ext/gd/tests/bug66356.phpt
+++ b/ext/gd/tests/bug66356.phpt
@@ -24,6 +24,8 @@ var_dump(imagecrop($img, array("x" => -20, "y" => -20, "width" => 10, "height" =
// POC #4
var_dump(imagecrop($img, array("x" => 0x7fffff00, "y" => 0, "width" => 10, "height" => 10)));
+// bug 66815
+var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => 65535, "height" => 65535)));
?>
--EXPECTF--
resource(%d) of type (gd)
@@ -35,6 +37,13 @@ Array
[width] => 10
[height] => 10
)
+
+Warning: imagecrop(): gd warning: one parameter to a memory allocation multiplication is negative or zero, failing operation gracefully
+ in %sbug66356.php on line %d
bool(false)
resource(%d) of type (gd)
-resource(%d) of type (gd) \ No newline at end of file
+resource(%d) of type (gd)
+
+Warning: imagecrop(): gd warning: product of memory allocation multiplication would exceed INT_MAX, failing operation gracefully
+ in %sbug66356.php on line %d
+bool(false) \ No newline at end of file