From 3b52c47ba3d79a6a8451064c187e0b431f9240f6 Mon Sep 17 00:00:00 2001 From: Jan Oravec Date: Mon, 15 Aug 2016 13:17:58 -0700 Subject: [PATCH] Fix some color related crashes in libgd Summary: Various paths use the color as an index into a 256 element table without checking it, and passing -7 as a color to imageline would unconditionally cause it to treat the image as truecolor, seg-faulting if it wasn't. Reviewed By: alexmalyshev Differential Revision: D3658241 fbshipit-source-id: 0f4a9f32dce0a733b5749451b239badd0bfc4d49 --- hphp/runtime/ext/gd/ext_gd.cpp | 5 +- hphp/runtime/ext/gd/libgd/gd.cpp | 8 ++ hphp/runtime/ext/gd/libgd/gd_crop.cpp | 146 +++++++++++--------- hphp/runtime/ext/gd/libgd/gd_gd2.cpp | 4 +- hphp/runtime/ext/gd/libgd/gd_topal.cpp | 4 + hphp/test/slow/ext_gd/colorcrash.php | 8 ++ hphp/test/slow/ext_gd/colorcrash.php.expect | 1 + 7 files changed, 103 insertions(+), 73 deletions(-) create mode 100644 hphp/test/slow/ext_gd/colorcrash.php create mode 100644 hphp/test/slow/ext_gd/colorcrash.php.expect diff --git a/hphp/runtime/ext/gd/ext_gd.cpp b/hphp/runtime/ext/gd/ext_gd.cpp index c9cce80b471237..8fbe18b466deeb 100644 --- a/hphp/runtime/ext/gd/ext_gd.cpp +++ b/hphp/runtime/ext/gd/ext_gd.cpp @@ -3802,9 +3802,8 @@ Variant HHVM_FUNCTION(imagecolorsforindex, const Resource& image, gdImagePtr im = cast(image)->get(); if (!im) return false; #if HAVE_LIBGD20 - if ((index >= 0 && gdImageTrueColor(im)) || - (!gdImageTrueColor(im) && index >= 0 && - index < gdImageColorsTotal(im))) { + if (index >= 0 && + (gdImageTrueColor(im) || index < gdImageColorsTotal(im))) { return make_map_array( s_red, gdImageRed(im,index), s_green, gdImageGreen(im,index), diff --git a/hphp/runtime/ext/gd/libgd/gd.cpp b/hphp/runtime/ext/gd/libgd/gd.cpp index 8d965a0549aef3..839add37a11f3d 100644 --- a/hphp/runtime/ext/gd/libgd/gd.cpp +++ b/hphp/runtime/ext/gd/libgd/gd.cpp @@ -1041,6 +1041,12 @@ void gdImageAABlend (gdImagePtr im) int p_color, p_red, p_green, p_blue; int px, py; + if (!gdImageTrueColor(im) && color >= gdImageColorsTotal(im)) { + color = gdImageColorsTotal(im) - 1; + } + if (color < 0) { + color = 0; + } color_red = gdImageRed(im, color); color_green = gdImageGreen(im, color); color_blue = gdImageBlue(im, color); @@ -1314,6 +1320,8 @@ inline static void gdImageSetAAPixelColor(gdImagePtr im, int x, int y, int color **/ void gdImageAALine (gdImagePtr im, int x1, int y1, int x2, int y2, int col) { + if (!gdImageTrueColor(im)) return; + /* keep them as 32bits */ long x, y, inc; long dx, dy,tmp; diff --git a/hphp/runtime/ext/gd/libgd/gd_crop.cpp b/hphp/runtime/ext/gd/libgd/gd_crop.cpp index 6ee49c78e48b83..c6d6ced8b57505 100644 --- a/hphp/runtime/ext/gd/libgd/gd_crop.cpp +++ b/hphp/runtime/ext/gd/libgd/gd_crop.cpp @@ -213,75 +213,83 @@ gdImagePtr gdImageCropAuto(gdImagePtr im, const unsigned int mode) * See also: * , or */ -gdImagePtr gdImageCropThreshold(gdImagePtr im, const unsigned int color, const float threshold) -{ - const int width = gdImageSX(im); - const int height = gdImageSY(im); - - int x,y; - int match; - gdRect crop; - - crop.x = 0; - crop.y = 0; - crop.width = 0; - crop.height = 0; - - /* Pierre: crop everything sounds bad */ - if (threshold > 1.0) { - return NULL; - } - - /* TODO: Add gdImageGetRowPtr and works with ptr at the row level - * for the true color and palette images - * new formats will simply work with ptr - */ - match = 1; - for (y = 0; match && y < height; y++) { - for (x = 0; match && x < width; x++) { - match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0; - } - } - - /* Pierre - * Nothing to do > bye - * Duplicate the image? - */ - if (y == height - 1) { - return NULL; - } - - crop.y = y -1; - match = 1; - for (y = height - 1; match && y >= 0; y--) { - for (x = 0; match && x < width; x++) { - match = (gdColorMatch(im, color, gdImageGetPixel(im, x, y), threshold)) > 0; - } - } - - if (y == 0) { - crop.height = height - crop.y + 1; - } else { - crop.height = y - crop.y + 2; - } - - match = 1; - for (x = 0; match && x < width; x++) { - for (y = 0; match && y < crop.y + crop.height - 1; y++) { - match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0; - } - } - crop.x = x - 1; - - match = 1; - for (x = width - 1; match && x >= 0; x--) { - for (y = 0; match && y < crop.y + crop.height - 1; y++) { - match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0; - } - } - crop.width = x - crop.x + 2; - - return gdImageCrop(im, &crop); +gdImagePtr gdImageCropThreshold(gdImagePtr im, const unsigned int color, + const float threshold) { + const int width = gdImageSX(im); + const int height = gdImageSY(im); + + int x,y; + int match; + gdRect crop; + + crop.x = 0; + crop.y = 0; + crop.width = 0; + crop.height = 0; + + /* Pierre: crop everything sounds bad */ + if (threshold > 1.0) { + return nullptr; + } + + if (!gdImageTrueColor(im) && color > gdImageColorsTotal(im)) { + return nullptr; + } + + /* TODO: Add gdImageGetRowPtr and works with ptr at the row level + * for the true color and palette images + * new formats will simply work with ptr + */ + match = 1; + for (y = 0; match && y < height; y++) { + for (x = 0; match && x < width; x++) { + match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), + threshold)) > 0; + } + } + + /* Pierre + * Nothing to do > bye + * Duplicate the image? + */ + if (y == height - 1) { + return nullptr; + } + + crop.y = y -1; + match = 1; + for (y = height - 1; match && y >= 0; y--) { + for (x = 0; match && x < width; x++) { + match = + (gdColorMatch(im, color, gdImageGetPixel(im, x, y), threshold)) > 0; + } + } + + if (y == 0) { + crop.height = height - crop.y + 1; + } else { + crop.height = y - crop.y + 2; + } + + match = 1; + for (x = 0; match && x < width; x++) { + for (y = 0; match && y < crop.y + crop.height - 1; y++) { + match = + (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0; + } + } + crop.x = x - 1; + + match = 1; + for (x = width - 1; match && x >= 0; x--) { + for (y = 0; match && y < crop.y + crop.height - 1; y++) { + match = + (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0; + } + } + crop.width = x - crop.x + 2; + + return gdImageCrop(im, &crop); } /* This algorithm comes from pnmcrop (http://netpbm.sourceforge.net/) diff --git a/hphp/runtime/ext/gd/libgd/gd_gd2.cpp b/hphp/runtime/ext/gd/libgd/gd_gd2.cpp index 7435667d31529b..94b827eae7b490 100644 --- a/hphp/runtime/ext/gd/libgd/gd_gd2.cpp +++ b/hphp/runtime/ext/gd/libgd/gd_gd2.cpp @@ -666,10 +666,12 @@ static void _gdImageGd2 (gdImagePtr im, gdIOCtx * out, int cs, int fmt) /* Force fmt to a valid value since we don't return anything. */ if ((fmt != GD2_FMT_RAW) && (fmt != GD2_FMT_COMPRESSED)) { - fmt = im->trueColor ? GD2_FMT_TRUECOLOR_COMPRESSED : GD2_FMT_COMPRESSED; + fmt = GD2_FMT_COMPRESSED; } if (im->trueColor) { fmt += 2; + assert(fmt == GD2_FMT_TRUECOLOR_RAW || + fmt == GD2_FMT_TRUECOLOR_COMPRESSED); } /* Make sure chunk size is valid. These are arbitrary values; 64 because it seems * a little silly to expect performance improvements on a 64x64 bit scale, and diff --git a/hphp/runtime/ext/gd/libgd/gd_topal.cpp b/hphp/runtime/ext/gd/libgd/gd_topal.cpp index 9354ce46897b70..b9cd0243930d1e 100644 --- a/hphp/runtime/ext/gd/libgd/gd_topal.cpp +++ b/hphp/runtime/ext/gd/libgd/gd_topal.cpp @@ -1818,6 +1818,10 @@ static void gdImageTrueColorToPaletteBody (gdImagePtr oim, int dither, int color { colorsWanted = maxColors; } + if (colorsWanted <= 0) { + colorsWanted = 1; + } + if (!cimP) { nim->pixels = (unsigned char **) gdCalloc (sizeof (unsigned char *), oim->sy); if (!nim->pixels) diff --git a/hphp/test/slow/ext_gd/colorcrash.php b/hphp/test/slow/ext_gd/colorcrash.php new file mode 100644 index 00000000000000..dfc9aaa532a3c0 --- /dev/null +++ b/hphp/test/slow/ext_gd/colorcrash.php @@ -0,0 +1,8 @@ +