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
This commit is contained in:
dml 2015-03-18 06:03:29 -07:00 committed by Commit bot
parent 8188840141
commit 78acf96a67
3 changed files with 63 additions and 14 deletions

View File

@ -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',

View File

@ -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);

View File

@ -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);
}