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
This commit is contained in:
parent
9552662e9f
commit
493c1ce1cd
@ -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',
|
||||
|
@ -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);
|
||||
|
44
tests/IndexedPngOverflowTest.cpp
Normal file
44
tests/IndexedPngOverflowTest.cpp
Normal 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);
|
||||
}
|
Loading…
Reference in New Issue
Block a user