aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--resources/invalid_images/b33251605.bmpbin0 -> 125 bytes
-rw-r--r--src/codec/SkBmpCodec.cpp3
-rw-r--r--src/codec/SkBmpRLECodec.cpp92
-rw-r--r--src/codec/SkBmpRLECodec.h13
-rw-r--r--tests/CodecTest.cpp12
5 files changed, 56 insertions, 64 deletions
diff --git a/resources/invalid_images/b33251605.bmp b/resources/invalid_images/b33251605.bmp
new file mode 100644
index 0000000000..0060ff48dd
--- /dev/null
+++ b/resources/invalid_images/b33251605.bmp
Binary files differ
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 83ba21b0d6..66ad0caa2b 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -549,7 +549,6 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
SkCodecPrintf("Error: RLE requires valid input size.\n");
return false;
}
- const size_t RLEBytes = totalBytes - offset;
// Bmp-in-Ico must be standard mode
// When inIco is true, this line cannot be reached, since we
@@ -565,7 +564,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
const SkEncodedInfo info = SkEncodedInfo::Make(SkEncodedInfo::kBGRA_Color,
SkEncodedInfo::kBinary_Alpha, 8);
*codecOut = new SkBmpRLECodec(width, height, info, stream, bitsPerPixel, numColors,
- bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
+ bytesPerColor, offset - bytesRead, rowOrder);
}
return true;
}
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index c2574ddef8..1968616fa6 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -17,16 +17,13 @@
SkBmpRLECodec::SkBmpRLECodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
uint16_t bitsPerPixel, uint32_t numColors,
uint32_t bytesPerColor, uint32_t offset,
- SkCodec::SkScanlineOrder rowOrder,
- size_t RLEBytes)
+ SkCodec::SkScanlineOrder rowOrder)
: INHERITED(width, height, info, stream, bitsPerPixel, rowOrder)
, fColorTable(nullptr)
, fNumColors(numColors)
, fBytesPerColor(bytesPerColor)
, fOffset(offset)
- , fStreamBuffer(new uint8_t[RLEBytes])
- , fRLEBytes(RLEBytes)
- , fOrigRLEBytes(RLEBytes)
+ , fBytesBuffered(0)
, fCurrRLEByte(0)
, fSampleX(1)
{}
@@ -134,22 +131,8 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo,
}
bool SkBmpRLECodec::initializeStreamBuffer() {
- // Setup a buffer to contain the full input stream
- // TODO (msarett): I'm not sure it is smart or optimal to trust fRLEBytes (read from header)
- // as the size of our buffer. First of all, the decode fails if fRLEBytes is
- // corrupt (negative, zero, or small) when we might be able to decode
- // successfully with a fixed size buffer. Additionally, we would save memory
- // using a fixed size buffer if the RLE encoding is large. On the other hand,
- // we may also waste memory with a fixed size buffer. And determining a
- // minimum size for our buffer would depend on the image width (so it's not
- // really "fixed" size), and we may end up allocating a buffer that is
- // generally larger than the average encoded size anyway.
- size_t totalBytes = this->stream()->read(fStreamBuffer.get(), fRLEBytes);
- if (totalBytes < fRLEBytes) {
- fRLEBytes = totalBytes;
- SkCodecPrintf("Warning: incomplete RLE file.\n");
- }
- if (fRLEBytes == 0) {
+ fBytesBuffered = this->stream()->read(fStreamBuffer, kBufferSize);
+ if (fBytesBuffered == 0) {
SkCodecPrintf("Error: could not read RLE image data.\n");
return false;
}
@@ -158,15 +141,12 @@ bool SkBmpRLECodec::initializeStreamBuffer() {
}
/*
- * Before signalling kIncompleteInput, we should attempt to load the
- * stream buffer with additional data.
- *
* @return the number of bytes remaining in the stream buffer after
* attempting to read more bytes from the stream
*/
size_t SkBmpRLECodec::checkForMoreData() {
- const size_t remainingBytes = fRLEBytes - fCurrRLEByte;
- uint8_t* buffer = fStreamBuffer.get();
+ const size_t remainingBytes = fBytesBuffered - fCurrRLEByte;
+ uint8_t* buffer = fStreamBuffer;
// We will be reusing the same buffer, starting over from the beginning.
// Move any remaining bytes to the start of the buffer.
@@ -185,11 +165,8 @@ size_t SkBmpRLECodec::checkForMoreData() {
// Update counters and return the number of bytes we currently have
// available. We are at the start of the buffer again.
fCurrRLEByte = 0;
- // If we were unable to fill the buffer, fRLEBytes is no longer equal to
- // the size of the buffer. There will be unused space at the end. This
- // should be fine, given that there are no more bytes in the stream.
- fRLEBytes = remainingBytes + additionalBytes;
- return fRLEBytes;
+ fBytesBuffered = remainingBytes + additionalBytes;
+ return fBytesBuffered;
}
/*
@@ -294,7 +271,6 @@ SkCodec::Result SkBmpRLECodec::onPrepareToDecode(const SkImageInfo& dstInfo,
copy_color_table(dstInfo, fColorTable.get(), inputColorPtr, inputColorCount);
// Initialize a buffer for encoded RLE data
- fRLEBytes = fOrigRLEBytes;
if (!this->initializeStreamBuffer()) {
SkCodecPrintf("Error: cannot initialize stream buffer.\n");
return SkCodec::kInvalidInput;
@@ -392,8 +368,7 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
}
// Every entry takes at least two bytes
- if ((int) fRLEBytes - fCurrRLEByte < 2) {
- SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+ if ((int) fBytesBuffered - fCurrRLEByte < 2) {
if (this->checkForMoreData() < 2) {
return y;
}
@@ -403,8 +378,8 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
// depending on their values. In the first interpretation, the first
// byte is an escape flag and the second byte indicates what special
// task to perform.
- const uint8_t flag = fStreamBuffer.get()[fCurrRLEByte++];
- const uint8_t task = fStreamBuffer.get()[fCurrRLEByte++];
+ const uint8_t flag = fStreamBuffer[fCurrRLEByte++];
+ const uint8_t task = fStreamBuffer[fCurrRLEByte++];
// Perform decoding
if (RLE_ESCAPE == flag) {
@@ -417,15 +392,14 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
return height;
case RLE_DELTA: {
// Two bytes are needed to specify delta
- if ((int) fRLEBytes - fCurrRLEByte < 2) {
- SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+ if ((int) fBytesBuffered - fCurrRLEByte < 2) {
if (this->checkForMoreData() < 2) {
return y;
}
}
// Modify x and y
- const uint8_t dx = fStreamBuffer.get()[fCurrRLEByte++];
- const uint8_t dy = fStreamBuffer.get()[fCurrRLEByte++];
+ const uint8_t dx = fStreamBuffer[fCurrRLEByte++];
+ const uint8_t dy = fStreamBuffer[fCurrRLEByte++];
x += dx;
y += dy;
if (x > width) {
@@ -451,11 +425,20 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
SkCodecPrintf("Warning: invalid RLE input.\n");
return y;
}
+
// Also abort if there are not enough bytes
// remaining in the stream to set numPixels.
- if ((int) fRLEBytes - fCurrRLEByte < SkAlign2(rowBytes)) {
- SkCodecPrintf("Warning: might be incomplete RLE input.\n");
- if (this->checkForMoreData() < SkAlign2(rowBytes)) {
+
+ // At most, alignedRowBytes can be 255 (max uint8_t) *
+ // 3 (max bytes per pixel) + 1 (aligned) = 766. If
+ // fStreamBuffer was smaller than this,
+ // checkForMoreData would never succeed for some bmps.
+ static_assert(255 * 3 + 1 < kBufferSize,
+ "kBufferSize needs to be larger!");
+ const size_t alignedRowBytes = SkAlign2(rowBytes);
+ if ((int) fBytesBuffered - fCurrRLEByte < alignedRowBytes) {
+ SkASSERT(alignedRowBytes < kBufferSize);
+ if (this->checkForMoreData() < alignedRowBytes) {
return y;
}
}
@@ -463,8 +446,8 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
while (numPixels > 0) {
switch(this->bitsPerPixel()) {
case 4: {
- SkASSERT(fCurrRLEByte < fRLEBytes);
- uint8_t val = fStreamBuffer.get()[fCurrRLEByte++];
+ SkASSERT(fCurrRLEByte < fBytesBuffered);
+ uint8_t val = fStreamBuffer[fCurrRLEByte++];
setPixel(dst, dstRowBytes, dstInfo, x++,
y, val >> 4);
numPixels--;
@@ -476,16 +459,16 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
break;
}
case 8:
- SkASSERT(fCurrRLEByte < fRLEBytes);
+ SkASSERT(fCurrRLEByte < fBytesBuffered);
setPixel(dst, dstRowBytes, dstInfo, x++,
- y, fStreamBuffer.get()[fCurrRLEByte++]);
+ y, fStreamBuffer[fCurrRLEByte++]);
numPixels--;
break;
case 24: {
- SkASSERT(fCurrRLEByte + 2 < fRLEBytes);
- uint8_t blue = fStreamBuffer.get()[fCurrRLEByte++];
- uint8_t green = fStreamBuffer.get()[fCurrRLEByte++];
- uint8_t red = fStreamBuffer.get()[fCurrRLEByte++];
+ SkASSERT(fCurrRLEByte + 2 < fBytesBuffered);
+ uint8_t blue = fStreamBuffer[fCurrRLEByte++];
+ uint8_t green = fStreamBuffer[fCurrRLEByte++];
+ uint8_t red = fStreamBuffer[fCurrRLEByte++];
setRGBPixel(dst, dstRowBytes, dstInfo,
x++, y, red, green, blue);
numPixels--;
@@ -513,8 +496,7 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
// In RLE24, the second byte read is part of the pixel color.
// There are two more required bytes to finish encoding the
// color.
- if ((int) fRLEBytes - fCurrRLEByte < 2) {
- SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+ if ((int) fBytesBuffered - fCurrRLEByte < 2) {
if (this->checkForMoreData() < 2) {
return y;
}
@@ -522,8 +504,8 @@ int SkBmpRLECodec::decodeRLE(const SkImageInfo& dstInfo, void* dst, size_t dstRo
// Fill the pixels up to endX with the specified color
uint8_t blue = task;
- uint8_t green = fStreamBuffer.get()[fCurrRLEByte++];
- uint8_t red = fStreamBuffer.get()[fCurrRLEByte++];
+ uint8_t green = fStreamBuffer[fCurrRLEByte++];
+ uint8_t red = fStreamBuffer[fCurrRLEByte++];
while (x < endX) {
setRGBPixel(dst, dstRowBytes, dstInfo, x++, y, red, green, blue);
}
diff --git a/src/codec/SkBmpRLECodec.h b/src/codec/SkBmpRLECodec.h
index 7cb3e9b292..8ea3a86dba 100644
--- a/src/codec/SkBmpRLECodec.h
+++ b/src/codec/SkBmpRLECodec.h
@@ -32,13 +32,10 @@ public:
* @param offset the offset of the image pixel data from the end of the
* headers
* @param rowOrder indicates whether rows are ordered top-down or bottom-up
- * @param RLEBytes indicates the amount of data left in the stream
- * after decoding the headers
*/
SkBmpRLECodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
uint16_t bitsPerPixel, uint32_t numColors, uint32_t bytesPerColor,
- uint32_t offset, SkCodec::SkScanlineOrder rowOrder,
- size_t RLEBytes);
+ uint32_t offset, SkCodec::SkScanlineOrder rowOrder);
int setSampleX(int);
@@ -100,9 +97,11 @@ private:
const uint32_t fNumColors;
const uint32_t fBytesPerColor;
const uint32_t fOffset;
- std::unique_ptr<uint8_t[]> fStreamBuffer;
- size_t fRLEBytes;
- const size_t fOrigRLEBytes;
+
+ static constexpr size_t kBufferSize = 4096;
+ uint8_t fStreamBuffer[kBufferSize];
+ size_t fBytesBuffered;
+
uint32_t fCurrRLEByte;
int fSampleX;
std::unique_ptr<SkSampler> fSampler;
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 0f6d54c8d7..8294c7a565 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1448,6 +1448,18 @@ DEF_TEST(Codec_InvalidBmp, r) {
REPORTER_ASSERT(r, !codec);
}
+DEF_TEST(Codec_InvalidRLEBmp, r) {
+ auto* stream = GetResourceAsStream("invalid_images/b33251605.bmp");
+ if (!stream) {
+ return;
+ }
+
+ std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream));
+ REPORTER_ASSERT(r, codec);
+
+ test_info(r, codec.get(), codec->getInfo(), SkCodec::kIncompleteInput, nullptr);
+}
+
DEF_TEST(Codec_InvalidAnimated, r) {
// ASAN will complain if there is an issue.
auto path = "invalid_images/skbug6046.gif";
generated by cgit v1.2.3 (git 2.39.1) at 2024-05-25 04:56:06 +0000