Indexed PNG decoding: Ensure color table is large enough that the bit depth of the image will not a…

src/images/SkImageDecoder_libpng.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkImageDecoder.h"
 #include "SkImageEncoder.h"
 #include "SkColor.h"
@@ skipping 81 matching lines @@
     bool onDecodeSubset(SkBitmap* bitmap, const SkIRect& region) SK_OVERRIDE;
 #endif
     Result onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
 
 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 * SK_RESTRICT hasAlphap, bool *reallyHasAlphap,
-                       SkColorTable **colorTablep);
+                       SkColorTable **colorTablep, int bitDepth);

[scroggo, 2015/03/05 23:23:30]

nit: I would prefer to have this new input parameter before all these output
parameters.

[David Lattimore, 2015/03/06 00:15:26]

Done.

     bool getBitmapColorType(png_structp, png_infop, SkColorType*, bool* hasAlpha,
                             SkPMColor* theTranspColor);
 
     typedef SkImageDecoder INHERITED;
 };
 
 #ifndef png_jmpbuf
 #  define png_jmpbuf(png_ptr) ((png_ptr)->jmpbuf)
 #endif
 
@@ skipping 230 matching lines @@
 
     // from here down we are concerned with colortables and pixels
 
     // we track if we actually see a non-opaque pixels, since sometimes a PNG sets its colortype
     // to |= PNG_COLOR_MASK_ALPHA, but all of its pixels are in fact opaque. We care, since we
     // draw lots faster if we can flag the bitmap has being opaque
     bool reallyHasAlpha = false;
     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);
 
     if (!this->allocPixelRef(decodedBitmap,
                              kIndex_8_SkColorType == colorType ? colorTable : NULL)) {
         return kFailure;
     }
 
     SkAutoLockPixels alp(*decodedBitmap);
@@ skipping 281 matching lines @@
         png_set_filler(png_ptr, 0xff, PNG_FILLER_AFTER);
     }
 
     return true;
 }
 
 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,
-                                      SkColorTable **colorTablep) {
+                                      SkColorTable **colorTablep, int bitDepth) {
     int numPalette;
     png_colorp palette;
     png_bytep trans;
     int numTrans;
 
     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;
 
     if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
         png_get_tRNS(png_ptr, info_ptr, &trans, &numTrans, NULL);
         *hasAlphap = (numTrans > 0);
     } else {
         numTrans = 0;
     }
 
@@ skipping 18 matching lines @@
         *colorPtr++ = proc(*trans++, palette->red, palette->green, palette->blue);
         palette++;
     }
     bool reallyHasAlpha = (transLessThanFF < 0);
 
     for (; index < numPalette; index++) {
         *colorPtr++ = SkPackARGB32(0xFF, palette->red, palette->green, palette->blue);
         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));
     *reallyHasAlphap = reallyHasAlpha;
     return true;
 }
 
 #ifdef SK_BUILD_FOR_ANDROID
 
 bool SkPNGImageDecoder::onBuildTileIndex(SkStreamRewindable* sk_stream, int *width, int *height) {
@@ skipping 69 matching lines @@
 
     // from here down we are concerned with colortables and pixels
 
     // we track if we actually see a non-opaque pixels, since sometimes a PNG sets its colortype
     // to |= PNG_COLOR_MASK_ALPHA, but all of its pixels are in fact opaque. We care, since we
     // draw lots faster if we can flag the bitmap has being opaque
     bool reallyHasAlpha = false;
     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);
 
     // Check ahead of time if the swap(dest, src) is possible.
     // If yes, then we will stick to AllocPixelRef since it's cheaper with the swap happening.
     // If no, then we will use alloc to allocate pixels to prevent garbage collection.
     int w = rect.width() / sampleSize;
     int h = rect.height() / sampleSize;
     const bool swapOnly = (rect == region) && (w == decodedBitmap.width()) &&
@@ skipping 459 matching lines @@
     return SkImageDecoder::kUnknown_Format;
 }
 
 SkImageEncoder* sk_libpng_efactory(SkImageEncoder::Type t) {
     return (SkImageEncoder::kPNG_Type == t) ? SkNEW(SkPNGImageEncoder) : NULL;
 }
 
 static SkImageDecoder_DecodeReg gDReg(sk_libpng_dfactory);
 static SkImageDecoder_FormatReg gFormatReg(get_format_png);
 static SkImageEncoder_EncodeReg gEReg(sk_libpng_efactory);