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, 63 insertions, 14 deletions
diff --git a/gyp/tests.gypi b/gyp/tests.gypi index d4ca3af476..f03488d95e 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -126,6 +126,7 @@ '../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 c074268cff..acaeda538d 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, + bool decodePalette(png_structp png_ptr, png_infop info_ptr, int bitDepth, 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, &hasAlpha, &reallyHasAlpha, &colorTable); + decodePalette(png_ptr, info_ptr, bitDepth, &hasAlpha, &reallyHasAlpha, &colorTable); } SkAutoUnref aur(colorTable); @@ -657,7 +657,8 @@ 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, - bool *hasAlphap, bool *reallyHasAlphap, + int bitDepth, bool *hasAlphap, + bool *reallyHasAlphap, SkColorTable **colorTablep) { int numPalette; png_colorp palette; @@ -666,13 +667,6 @@ 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; @@ -711,9 +705,19 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr, palette++; } - // see BUGGY IMAGE WORKAROUND comment above - if (numPalette < 256) { - *colorPtr = colorPtr[-1]; + /* 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; } *colorTablep = SkNEW_ARGS(SkColorTable, (colorStorage, colorCount)); @@ -803,7 +807,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); + decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable, bitDepth); } SkAutoUnref aur(colorTable); diff --git a/tests/IndexedPngOverflowTest.cpp b/tests/IndexedPngOverflowTest.cpp new file mode 100644 index 0000000000..914053a3ac --- /dev/null +++ b/tests/IndexedPngOverflowTest.cpp @@ -0,0 +1,44 @@ +/* + * 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); +} |