diff options
author | 2015-03-17 08:14:07 -0400 | |
---|---|---|
committer | 2015-03-17 08:14:07 -0400 | |
commit | 493c1ce1cd406ef28683203146274154783452ce (patch) | |
tree | d3f646e6e959cc95925d986daf370280bcd3a0dd /src/images | |
parent | 9552662e9fee5eb0ef435e52ab9db505d7ebe4ad (diff) |
Indexed PNG decoding: Ensure color table is large enough that the bit depth of the image will not allow reads beyond its end.
BUG=skia:3440
R=rmistry@google.com, scroggo@google.com
Review URL: https://codereview.chromium.org/948163002
Diffstat (limited to 'src/images')
-rw-r--r-- | src/images/SkImageDecoder_libpng.cpp | 32 |
1 files changed, 18 insertions, 14 deletions
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); |