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

Committed: https://skia.googlesource.com/skia/+/493c1ce1cd406ef28683203146274154783452ce

Review URL: https://codereview.chromium.org/948163002

1 files changed, 18 insertions, 14 deletions

diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp
index c074268cff..245654dbfd 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, bitDepth, &hasAlpha, &reallyHasAlpha, &colorTable);
     }
 
     SkAutoUnref aur(colorTable);