AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Indexed PNG decoding: Ensure color table is large enough that the bit depth of the image will not allow reads beyond its end."

Browse Source 
This reverts commit 493c1ce1cd.

NOTRY=true
NOTREECHECKS=true
TBR=egdaniel@google.com,dml@google.com
BUG=skia:

Review URL: https://codereview.chromium.org/1014553003
This commit is contained in:
scroggo 2015-03-17 05:25:54 -07:00 committed by Commit bot
parent 493c1ce1cd
commit cdeca44619
3 changed files with 14 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gyp/tests.gypi
View File

@ -126,7 +126,6 @@
'../tests/ImageGeneratorTest.cpp',
'../tests/ImageIsOpaqueTest.cpp',
'../tests/ImageNewShaderTest.cpp',
'../tests/IndexedPngOverflowTest.cpp',
'../tests/InfRectTest.cpp',
'../tests/InterpolatorTest.cpp',
'../tests/InvalidIndexedPngTest.cpp',

32
src/images/SkImageDecoder_libpng.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, int bitDepth,
bool decodePalette(png_structp png_ptr, png_infop info_ptr,
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, bitDepth, &hasAlpha, &reallyHasAlpha, &colorTable);
decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable);
}
SkAutoUnref aur(colorTable);
@ -657,8 +657,7 @@ 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,
int bitDepth, bool *hasAlphap,
bool *reallyHasAlphap,
bool *hasAlphap, bool *reallyHasAlphap,
SkColorTable **colorTablep) {
int numPalette;
png_colorp palette;
@ -667,6 +666,13 @@ 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,19 +711,9 @@ bool SkPNGImageDecoder::decodePalette(png_structp png_ptr, png_infop info_ptr,
palette++;
}
/* 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;
// see BUGGY IMAGE WORKAROUND comment above
if (numPalette < 256) {
*colorPtr = colorPtr[-1];
}
*colorTablep = SkNEW_ARGS(SkColorTable, (colorStorage, colorCount));
@ -807,7 +803,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, bitDepth);
decodePalette(png_ptr, info_ptr, &hasAlpha, &reallyHasAlpha, &colorTable);
}
SkAutoUnref aur(colorTable);

44
tests/IndexedPngOverflowTest.cpp
View File

@ -1,44 +0,0 @@
/*
* 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);
}