Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(22)

Unified Diff: src/images/SkImageDecoder_libpng.cpp

Issue 948163002: Indexed PNG decoding: Ensure color table is large enough that the bit depth of the image will not a… (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Fixed argument order in some conditional Android code. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « gyp/tests.gypi ('k') | tests/IndexedPngOverflowTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/images/SkImageDecoder_libpng.cpp
diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp
index f9ef6b7942306016c714668555d96b7c3c31fbb3..391139a9ff3581b180c9dbdf686767c49808012d 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);
@@ -651,7 +651,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;
@@ -660,13 +661,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;
@@ -705,9 +699,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));
@@ -797,7 +801,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);
« no previous file with comments | « gyp/tests.gypi ('k') | tests/IndexedPngOverflowTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698