Fix #77272: imagescale() may return image resource on failure

`_gdScaleHoriz()` and `_gdScaleVert()` may fail, but don't signal failure since they are void functions. We change that according to upstream libgd. We also remove the unused `Scale()` function, which doesn't exist in upstream libgd either, right away.
php · Jan 10, 2019 · 772b1cb · 772b1cb
1 parent 5d33024
commit 772b1cb
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 26 deletions.
diff --git a/NEWS b/NEWS
@@ -6,6 +6,7 @@ PHP                                                                        NEWS
   . Fixed bug #77339 (__callStatic may get incorrect arguments). (Dmitry)
 
 - GD:
+  . Fixed bug #77272 (imagescale() may return image resource on failure). (cmb)
   . Fixed bug #77391 (1bpp BMPs may fail to be loaded). (Romain Déoux, cmb)
 
 - MySQLnd:

diff --git a/ext/gd/libgd/gd_interpolation.c b/ext/gd/libgd/gd_interpolation.c
@@ -1021,7 +1021,7 @@ static inline void _gdScaleRow(gdImagePtr pSrc,  unsigned int src_width, gdImage
     }
 }
 
-static inline void _gdScaleHoriz(gdImagePtr pSrc, unsigned int src_width, unsigned int src_height, gdImagePtr pDst,  unsigned int dst_width, unsigned int dst_height)
+static inline int _gdScaleHoriz(gdImagePtr pSrc, unsigned int src_width, unsigned int src_height, gdImagePtr pDst,  unsigned int dst_width, unsigned int dst_height)
 {
 	unsigned int u;
 	LineContribType * contrib;
@@ -1036,13 +1036,14 @@ static inline void _gdScaleHoriz(gdImagePtr pSrc, unsigned int src_width, unsign
 
 	contrib = _gdContributionsCalc(dst_width, src_width, (double)dst_width / (double)src_width, pSrc->interpolation);
 	if (contrib == NULL) {
-		return;
+		return 0;
 	}
 	/* Scale each row */
 	for (u = 0; u < dst_height - 1; u++) {
 		_gdScaleRow(pSrc, src_width, pDst, dst_width, u, contrib);
 	}
 	_gdContributionsFree (contrib);
+	return 1;
 }
 
 static inline void _gdScaleCol (gdImagePtr pSrc,  unsigned int src_width, gdImagePtr pRes, unsigned int dst_width, unsigned int dst_height, unsigned int uCol, LineContribType *contrib)
@@ -1068,7 +1069,7 @@ static inline void _gdScaleCol (gdImagePtr pSrc,  unsigned int src_width, gdImag
 	}
 }
 
-static inline void _gdScaleVert (const gdImagePtr pSrc, const unsigned int src_width, const unsigned int src_height, const gdImagePtr pDst, const unsigned int dst_width, const unsigned int dst_height)
+static inline int _gdScaleVert (const gdImagePtr pSrc, const unsigned int src_width, const unsigned int src_height, const gdImagePtr pDst, const unsigned int dst_width, const unsigned int dst_height)
 {
 	unsigned int u;
 	LineContribType * contrib;
@@ -1083,19 +1084,21 @@ static inline void _gdScaleVert (const gdImagePtr pSrc, const unsigned int src_w
 
 	contrib = _gdContributionsCalc(dst_height, src_height, (double)(dst_height) / (double)(src_height), pSrc->interpolation);
 	if (contrib == NULL) {
-		return;
+		return 0;
 	}
 	/* scale each column */
 	for (u = 0; u < dst_width - 1; u++) {
 		_gdScaleCol(pSrc, src_width, pDst, dst_width, dst_height, u, contrib);
 	}
 	_gdContributionsFree(contrib);
+	return 1;
 }
 
 gdImagePtr gdImageScaleTwoPass(const gdImagePtr src, const unsigned int src_width, const unsigned int src_height, const unsigned int new_width, const unsigned int new_height)
 {
 	gdImagePtr tmp_im;
 	gdImagePtr dst;
+	int scale_pass_res;
 
 	if (new_width == 0 || new_height == 0) {
 		return NULL;
@@ -1111,38 +1114,26 @@ gdImagePtr gdImageScaleTwoPass(const gdImagePtr src, const unsigned int src_widt
 		return NULL;
 	}
 	gdImageSetInterpolationMethod(tmp_im, src->interpolation_id);
-	_gdScaleHoriz(src, src_width, src_height, tmp_im, new_width, src_height);
+	scale_pass_res = _gdScaleHoriz(src, src_width, src_height, tmp_im, new_width, src_height);
+	if (scale_pass_res != 1) {
+		gdImageDestroy(tmp_im);
+		return NULL;
+	}
 
 	dst = gdImageCreateTrueColor(new_width, new_height);
 	if (dst == NULL) {
 		gdImageDestroy(tmp_im);
 		return NULL;
 	}
 	gdImageSetInterpolationMethod(dst, src->interpolation_id);
-	_gdScaleVert(tmp_im, new_width, src_height, dst, new_width, new_height);
-	gdImageDestroy(tmp_im);
-
-	return dst;
-}
-
-gdImagePtr Scale(const gdImagePtr src, const unsigned int src_width, const unsigned int src_height, const gdImagePtr dst, const unsigned int new_width, const unsigned int new_height)
-{
-	gdImagePtr tmp_im;
-
-	if (new_width == 0 || new_height == 0) {
-		return NULL;
-	}
-
-	tmp_im = gdImageCreateTrueColor(new_width, src_height);
-	if (tmp_im == NULL) {
+	scale_pass_res = _gdScaleVert(tmp_im, new_width, src_height, dst, new_width, new_height);
+	if (scale_pass_res != 1) {
+		gdImageDestroy(dst);
+		gdImageDestroy(tmp_im);
 		return NULL;
 	}
-	gdImageSetInterpolationMethod(tmp_im, src->interpolation_id);
-
-	_gdScaleHoriz(src, src_width, src_height, tmp_im, new_width, src_height);
-	_gdScaleVert(tmp_im, new_width, src_height, dst, new_width, new_height);
-
 	gdImageDestroy(tmp_im);
+
 	return dst;
 }
 

diff --git a/ext/gd/tests/bug77272.phpt b/ext/gd/tests/bug77272.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #77272 (imagescale() may return image resource on failure)
+--INI--
+memory_limit=-1
+--SKIPIF--
+<?php
+if (!extension_loaded('gd')) die('skip gd extension not available');
+if (!GD_BUNGLED && version_compare(GD_VERSION, '2.2.5', '<=')) die('skip upstream fix not yet released');
+if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
+?>
+--FILE--
+<?php
+$img = imagecreate(2**28, 1);
+var_dump(imagescale($img, 1, 1, IMG_TRIANGLE));
+?>
+===DONE===
+--EXPECTF--
+Warning: imagescale():%S product of memory allocation multiplication would exceed INT_MAX, failing operation gracefully
+ in %s on line %d
+bool(false)
+===DONE===