diff options
| -rw-r--r-- | gyp/tests.gypi | 1 | ||||
| -rw-r--r-- | src/images/SkImageDecoder_libpng.cpp | 32 | ||||
| -rw-r--r-- | tests/IndexedPngOverflowTest.cpp | 44 |
3 files changed, 14 insertions, 63 deletions
diff --git a/gyp/tests.gypi b/gyp/tests.gypi index f03488d95e..d4ca3af476 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -126,7 +126,6 @@ '../tests/ImageGeneratorTest.cpp', '../tests/ImageIsOpaqueTest.cpp', '../tests/ImageNewShaderTest.cpp', - '../tests/IndexedPngOverflowTest.cpp', '../tests/InfRectTest.cpp', '../tests/InterpolatorTest.cpp', '../tests/InvalidIndexedPngTest.cpp', diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp index acaeda538d..c074268cff 100644 --- a/src/images/SkImageDecoder_libpng.cpp +++ b/src/images/SkImageDecoder_libpng.cpp @@ -97,7 +97,7 @@ private: SkPNGImageIndex* fImageIndex; bool onDecodeInit(SkStream* stream, png_structp *png_ptrp, png_infop *info_ptrp); - bool decodePalette(png_structp png_ptr, png_infop info_ptr, int bitDepth, + bool decodePalette(png_structp png_ptr, png_infop info_ptr, bool * SK_RESTRICT hasAlphap, bool *reallyHasAlphap, SkColorTable **colorTablep); bool getBitmapColorType(png_structp, png_infop, SkColorType*, bool* hasAlpha, @@ -350,7 +350,7 @@ SkImageDecoder::Result SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap SkColorTable* colorTable = NULL; if (pngColorType == PNG_COLOR_TYPE_PALETTE) { - decodePalette(png_ptr, info_ptr, bitDepth, &hasAlpha, &reallyHasAlpha, &colorTable); + decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable); } SkAutoUnref aur(colorTable); @@ -657,8 +657,7 @@ bool SkPNGImageDecoder::getBitmapColorType(png_structp png_ptr, png_infop info_p typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b); bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, - int bitDepth, bool *hasAlphap, - bool *reallyHasAlphap, + bool *hasAlphap, bool *reallyHasAlphap, SkColorTable **colorTablep) { int numPalette; png_colorp palette; @@ -667,6 +666,13 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, png_get_PLTE(png_ptr, info_ptr, &palette, &numPalette); + /* BUGGY IMAGE WORKAROUND + + We hit some images (e.g. fruit_.png) who contain bytes that are == colortable_count + which is a problem since we use the byte as an index. To work around this we grow + the colortable by 1 (if its < 256) and duplicate the last color into that slot. + */ + int colorCount = numPalette + (numPalette < 256); SkPMColor colorStorage[256]; // worst-case storage SkPMColor* colorPtr = colorStorage; @@ -705,19 +711,9 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, palette++; } - /* BUGGY IMAGE WORKAROUND - - Invalid images could contain pixel values that are greater than the number of palette - entries. Since we use pixel values as indices into the palette this could result in reading - beyond the end of the palette which could leak the contents of uninitialized memory. To - ensure this doesn't happen, we grow the colortable to the maximum size that can be - addressed by the bitdepth of the image and fill it with the last palette color or black if - the palette is empty (really broken image). - */ - int colorCount = SkTMax(numPalette, 1 << SkTMin(bitDepth, 8)); - SkPMColor lastColor = index > 0 ? colorPtr[-1] : SkPackARGB32(0xFF, 0, 0, 0); - for (; index < colorCount; index++) { - *colorPtr++ = lastColor; + // see BUGGY IMAGE WORKAROUND comment above + if (numPalette < 256) { + *colorPtr = colorPtr[-1]; } *colorTablep = SkNEW_ARGS(SkColorTable, (colorStorage, colorCount)); @@ -807,7 +803,7 @@ bool SkPNGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) { SkColorTable* colorTable = NULL; if (pngColorType == PNG_COLOR_TYPE_PALETTE) { - decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable, bitDepth); + decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable); } SkAutoUnref aur(colorTable); diff --git a/tests/IndexedPngOverflowTest.cpp b/tests/IndexedPngOverflowTest.cpp deleted file mode 100644 index 914053a3ac..0000000000 --- a/tests/IndexedPngOverflowTest.cpp +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2015 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "SkBitmap.h" -#include "SkCanvas.h" -#include "SkForceLinking.h" -#include "SkImageDecoder.h" -#include "SkImageInfo.h" -#include "SkSurface.h" -#include "Test.h" - -// A 20x1 image with 8 bits per pixel and a palette size of 2. Pixel values are 255, 254... Run -// this test with ASAN to make sure we don't try to access before/after any palette-sized buffers. -unsigned char gPng[] = { - 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, - 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00, 0x01, - 0x08, 0x03, 0x00, 0x00, 0x00, 0xe9, 0x4c, 0x7e, 0x17, 0x00, 0x00, 0x00, - 0x09, 0x70, 0x48, 0x59, 0x73, 0x00, 0x00, 0x00, 0x1c, 0x00, 0x00, 0x00, - 0x1c, 0x00, 0x0f, 0x01, 0xb9, 0x8f, 0x00, 0x00, 0x00, 0x06, 0x50, 0x4c, - 0x54, 0x45, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xa5, 0x67, 0xb9, 0xcf, - 0x00, 0x00, 0x00, 0x20, 0x49, 0x44, 0x41, 0x54, 0x78, 0xda, 0xed, 0xfd, - 0x07, 0x01, 0x00, 0x20, 0x08, 0x00, 0x41, 0xbc, 0x5b, 0xe8, 0xdf, 0x97, - 0x99, 0xe3, 0x92, 0xa0, 0xf2, 0xdf, 0x3d, 0x7b, 0x0d, 0xda, 0x04, 0x1c, - 0x03, 0xad, 0x00, 0x38, 0x5c, 0x2e, 0xad, 0x12, 0x00, 0x00, 0x00, 0x00, - 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82 -}; - -DEF_TEST(IndexedPngOverflow, reporter) { - SkBitmap image; - SkForceLinking(false); - bool success = SkImageDecoder::DecodeMemory( - gPng, sizeof(gPng), &image, SkColorType::kUnknown_SkColorType, - SkImageDecoder::kDecodePixels_Mode); - REPORTER_ASSERT(reporter, success); - - SkAutoTUnref<SkSurface> surface(SkSurface::NewRaster(SkImageInfo::MakeN32Premul(20, 1))); - SkCanvas* canvas = surface->getCanvas(); - SkRect destRect = SkRect::MakeXYWH(0, 0, 20, 1); - canvas->drawBitmapRect(image, destRect, NULL); -} |