From 886e5e41db5d6d42368f225785013c9308dc66bc Mon Sep 17 00:00:00 2001 From: benjaminwagner Date: Fri, 4 Dec 2015 08:48:26 -0800 Subject: [PATCH] Fix overflow caught by ASAN. BUG=skia: Review URL: https://codereview.chromium.org/1498923002 --- src/codec/SkBmpCodec.cpp | 13 ------------- src/codec/SkBmpCodec.h | 5 ----- src/codec/SkBmpRLECodec.cpp | 9 ++++++--- src/codec/SkBmpRLECodec.h | 1 + src/codec/SkBmpStandardCodec.cpp | 9 ++++++--- src/codec/SkBmpStandardCodec.h | 1 + 6 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp index aff5402..580ff25 100644 --- a/src/codec/SkBmpCodec.cpp +++ b/src/codec/SkBmpCodec.cpp @@ -550,19 +550,6 @@ int32_t SkBmpCodec::getDstRow(int32_t y, int32_t height) const { return height - y - 1; } -/* - * Compute the number of colors in the color table - */ -uint32_t SkBmpCodec::computeNumColors(uint32_t numColors) { - // Zero is a default for maxColors - // Also set numColors to maxColors when it is too large - uint32_t maxColors = 1 << fBitsPerPixel; - if (numColors == 0 || numColors >= maxColors) { - return maxColors; - } - return numColors; -} - SkCodec::Result SkBmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) { if (!conversion_possible(dstInfo, this->getInfo())) { diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h index 65662ff1..30523b0 100644 --- a/src/codec/SkBmpCodec.h +++ b/src/codec/SkBmpCodec.h @@ -81,11 +81,6 @@ protected: int32_t getDstRow(int32_t y, int32_t height) const; /* - * Compute the number of colors in the color table - */ - uint32_t computeNumColors(uint32_t numColors); - - /* * Accessors used by subclasses */ uint16_t bitsPerPixel() const { return fBitsPerPixel; } diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp index 4cee274..b010126 100644 --- a/src/codec/SkBmpRLECodec.cpp +++ b/src/codec/SkBmpRLECodec.cpp @@ -21,7 +21,7 @@ SkBmpRLECodec::SkBmpRLECodec(const SkImageInfo& info, SkStream* stream, size_t RLEBytes) : INHERITED(info, stream, bitsPerPixel, rowOrder) , fColorTable(nullptr) - , fNumColors(this->computeNumColors(numColors)) + , fNumColors(numColors) , fBytesPerColor(bytesPerColor) , fOffset(offset) , fStreamBuffer(new uint8_t[RLEBytes]) @@ -82,9 +82,12 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo, // access memory outside of our color table array. *numColors = maxColors; } + // Don't bother reading more than maxColors. + const uint32_t numColorsToRead = + fNumColors == 0 ? maxColors : SkTMin(fNumColors, maxColors); // Read the color table from the stream - colorBytes = fNumColors * fBytesPerColor; + colorBytes = numColorsToRead * fBytesPerColor; SkAutoTDeleteArray cBuffer(new uint8_t[colorBytes]); if (stream()->read(cBuffer.get(), colorBytes) != colorBytes) { SkCodecPrintf("Error: unable to read color table.\n"); @@ -93,7 +96,7 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo, // Fill in the color table uint32_t i = 0; - for (; i < fNumColors; i++) { + for (; i < numColorsToRead; i++) { uint8_t blue = get_byte(cBuffer.get(), i*fBytesPerColor); uint8_t green = get_byte(cBuffer.get(), i*fBytesPerColor + 1); uint8_t red = get_byte(cBuffer.get(), i*fBytesPerColor + 2); diff --git a/src/codec/SkBmpRLECodec.h b/src/codec/SkBmpRLECodec.h index fcf910d..df2a97d 100644 --- a/src/codec/SkBmpRLECodec.h +++ b/src/codec/SkBmpRLECodec.h @@ -90,6 +90,7 @@ private: SkSampler* getSampler(bool createIfNecessary) override; SkAutoTUnref fColorTable; // owned + // fNumColors is the number specified in the header, or 0 if not present in the header. const uint32_t fNumColors; const uint32_t fBytesPerColor; const uint32_t fOffset; diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp index 5bbdcd2..85b4077 100644 --- a/src/codec/SkBmpStandardCodec.cpp +++ b/src/codec/SkBmpStandardCodec.cpp @@ -20,7 +20,7 @@ SkBmpStandardCodec::SkBmpStandardCodec(const SkImageInfo& info, SkStream* stream SkCodec::SkScanlineOrder rowOrder, bool inIco) : INHERITED(info, stream, bitsPerPixel, rowOrder) , fColorTable(nullptr) - , fNumColors(this->computeNumColors(numColors)) + , fNumColors(numColors) , fBytesPerColor(bytesPerColor) , fOffset(offset) , fSwizzler(nullptr) @@ -80,9 +80,12 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, // access memory outside of our color table array. *numColors = maxColors; } + // Don't bother reading more than maxColors. + const uint32_t numColorsToRead = + fNumColors == 0 ? maxColors : SkTMin(fNumColors, maxColors); // Read the color table from the stream - colorBytes = fNumColors * fBytesPerColor; + colorBytes = numColorsToRead * fBytesPerColor; SkAutoTDeleteArray cBuffer(new uint8_t[colorBytes]); if (stream()->read(cBuffer.get(), colorBytes) != colorBytes) { SkCodecPrintf("Error: unable to read color table.\n"); @@ -110,7 +113,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, // Fill in the color table uint32_t i = 0; - for (; i < fNumColors; i++) { + for (; i < numColorsToRead; i++) { uint8_t blue = get_byte(cBuffer.get(), i*fBytesPerColor); uint8_t green = get_byte(cBuffer.get(), i*fBytesPerColor + 1); uint8_t red = get_byte(cBuffer.get(), i*fBytesPerColor + 2); diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h index 7f23616..b799900 100644 --- a/src/codec/SkBmpStandardCodec.h +++ b/src/codec/SkBmpStandardCodec.h @@ -83,6 +83,7 @@ private: void decodeIcoMask(SkStream* stream, const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes); SkAutoTUnref fColorTable; // owned + // fNumColors is the number specified in the header, or 0 if not present in the header. const uint32_t fNumColors; const uint32_t fBytesPerColor; const uint32_t fOffset; -- 2.7.4