Merge cherrypicks of [2413468, 2413357, 2413489, 2413431, 2413506, 2413398, 2413399, 2413319, 2413507, 2413508, 2413371, 2413410, 2413432, 2413358, 2413490, 2413359, 2413360, 2413320, 2413509, 2413491, 2414061, 2413372, 2413492, 2413433, 2413469, 2413411, 2414062, 2413373, 2413374, 2413470, 2413400, 2414063, 2414064, 2413447, 2413434, 2414101, 2413412] into nyc-mr2-release
Change-Id: I4f6e7e64dcd0d524cb9bb2978a9095db1f3fe6d3
diff --git a/resources/invalid_images/b33651913.bmp b/resources/invalid_images/b33651913.bmp
new file mode 100644
index 0000000..5999b56
--- /dev/null
+++ b/resources/invalid_images/b33651913.bmp
Binary files differ
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 7eb1baf..0aa2dad 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -79,6 +79,59 @@
return SkBmpCodec::NewFromStream(stream, true);
}
+// Header size constants
+static const uint32_t kBmpHeaderBytes = 14;
+static const uint32_t kBmpHeaderBytesPlusFour = kBmpHeaderBytes + 4;
+static const uint32_t kBmpOS2V1Bytes = 12;
+static const uint32_t kBmpOS2V2Bytes = 64;
+static const uint32_t kBmpInfoBaseBytes = 16;
+static const uint32_t kBmpInfoV1Bytes = 40;
+static const uint32_t kBmpInfoV2Bytes = 52;
+static const uint32_t kBmpInfoV3Bytes = 56;
+static const uint32_t kBmpInfoV4Bytes = 108;
+static const uint32_t kBmpInfoV5Bytes = 124;
+static const uint32_t kBmpMaskBytes = 12;
+
+static BmpHeaderType get_header_type(size_t infoBytes) {
+ if (infoBytes >= kBmpInfoBaseBytes) {
+ // Check the version of the header
+ switch (infoBytes) {
+ case kBmpInfoV1Bytes:
+ return kInfoV1_BmpHeaderType;
+ case kBmpInfoV2Bytes:
+ return kInfoV2_BmpHeaderType;
+ case kBmpInfoV3Bytes:
+ return kInfoV3_BmpHeaderType;
+ case kBmpInfoV4Bytes:
+ return kInfoV4_BmpHeaderType;
+ case kBmpInfoV5Bytes:
+ return kInfoV5_BmpHeaderType;
+ case 16:
+ case 20:
+ case 24:
+ case 28:
+ case 32:
+ case 36:
+ case 42:
+ case 46:
+ case 48:
+ case 60:
+ case kBmpOS2V2Bytes:
+ return kOS2VX_BmpHeaderType;
+ default:
+ SkCodecPrintf("Error: unknown bmp header format.\n");
+ return kUnknown_BmpHeaderType;
+ }
+ } if (infoBytes >= kBmpOS2V1Bytes) {
+ // The OS2V1 is treated separately because it has a unique format
+ return kOS2V1_BmpHeaderType;
+ } else {
+ // There are no valid bmp headers
+ SkCodecPrintf("Error: second bitmap header size is invalid.\n");
+ return kUnknown_BmpHeaderType;
+ }
+}
+
/*
* Read enough of the stream to initialize the SkBmpCodec. Returns a bool
* representing success or failure. If it returned true, and codecOut was
@@ -86,19 +139,6 @@
* Does *not* take ownership of the passed in SkStream.
*/
bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
- // Header size constants
- static const uint32_t kBmpHeaderBytes = 14;
- static const uint32_t kBmpHeaderBytesPlusFour = kBmpHeaderBytes + 4;
- static const uint32_t kBmpOS2V1Bytes = 12;
- static const uint32_t kBmpOS2V2Bytes = 64;
- static const uint32_t kBmpInfoBaseBytes = 16;
- static const uint32_t kBmpInfoV1Bytes = 40;
- static const uint32_t kBmpInfoV2Bytes = 52;
- static const uint32_t kBmpInfoV3Bytes = 56;
- static const uint32_t kBmpInfoV4Bytes = 108;
- static const uint32_t kBmpInfoV5Bytes = 124;
- static const uint32_t kBmpMaskBytes = 12;
-
// The total bytes in the bmp file
// We only need to use this value for RLE decoding, so we will only
// check that it is valid in the RLE case.
@@ -157,6 +197,12 @@
}
}
+ // Determine image information depending on second header format
+ const BmpHeaderType headerType = get_header_type(infoBytes);
+ if (kUnknown_BmpHeaderType == headerType) {
+ return false;
+ }
+
// We already read the first four bytes of the info header to get the size
const uint32_t infoBytesRemaining = infoBytes - 4;
@@ -182,80 +228,45 @@
// The image width and height
int width, height;
- // Determine image information depending on second header format
- BmpHeaderType headerType;
- if (infoBytes >= kBmpInfoBaseBytes) {
- // Check the version of the header
- switch (infoBytes) {
- case kBmpInfoV1Bytes:
- headerType = kInfoV1_BmpHeaderType;
- break;
- case kBmpInfoV2Bytes:
- headerType = kInfoV2_BmpHeaderType;
- break;
- case kBmpInfoV3Bytes:
- headerType = kInfoV3_BmpHeaderType;
- break;
- case kBmpInfoV4Bytes:
- headerType = kInfoV4_BmpHeaderType;
- break;
- case kBmpInfoV5Bytes:
- headerType = kInfoV5_BmpHeaderType;
- break;
- case 16:
- case 20:
- case 24:
- case 28:
- case 32:
- case 36:
- case 42:
- case 46:
- case 48:
- case 60:
- case kBmpOS2V2Bytes:
- headerType = kOS2VX_BmpHeaderType;
- break;
- default:
- // We do not signal an error here because there is the
- // possibility of new or undocumented bmp header types. Most
- // of the newer versions of bmp headers are similar to and
- // build off of the older versions, so we may still be able to
- // decode the bmp.
- SkCodecPrintf("Warning: unknown bmp header format.\n");
- headerType = kUnknown_BmpHeaderType;
- break;
- }
- // We check the size of the header before entering the if statement.
- // We should not reach this point unless the size is large enough for
- // these required fields.
- SkASSERT(infoBytesRemaining >= 12);
- width = get_int(iBuffer.get(), 0);
- height = get_int(iBuffer.get(), 4);
- bitsPerPixel = get_short(iBuffer.get(), 10);
+ switch (headerType) {
+ case kInfoV1_BmpHeaderType:
+ case kInfoV2_BmpHeaderType:
+ case kInfoV3_BmpHeaderType:
+ case kInfoV4_BmpHeaderType:
+ case kInfoV5_BmpHeaderType:
+ case kOS2VX_BmpHeaderType:
+ // We check the size of the header before entering the if statement.
+ // We should not reach this point unless the size is large enough for
+ // these required fields.
+ SkASSERT(infoBytesRemaining >= 12);
+ width = get_int(iBuffer.get(), 0);
+ height = get_int(iBuffer.get(), 4);
+ bitsPerPixel = get_short(iBuffer.get(), 10);
- // Some versions do not have these fields, so we check before
- // overwriting the default value.
- if (infoBytesRemaining >= 16) {
- compression = get_int(iBuffer.get(), 12);
- if (infoBytesRemaining >= 32) {
- numColors = get_int(iBuffer.get(), 28);
+ // Some versions do not have these fields, so we check before
+ // overwriting the default value.
+ if (infoBytesRemaining >= 16) {
+ compression = get_int(iBuffer.get(), 12);
+ if (infoBytesRemaining >= 32) {
+ numColors = get_int(iBuffer.get(), 28);
+ }
}
- }
- // All of the headers that reach this point, store color table entries
- // using 4 bytes per pixel.
- bytesPerColor = 4;
- } else if (infoBytes >= kBmpOS2V1Bytes) {
- // The OS2V1 is treated separately because it has a unique format
- headerType = kOS2V1_BmpHeaderType;
- width = (int) get_short(iBuffer.get(), 0);
- height = (int) get_short(iBuffer.get(), 2);
- bitsPerPixel = get_short(iBuffer.get(), 6);
- bytesPerColor = 3;
- } else {
- // There are no valid bmp headers
- SkCodecPrintf("Error: second bitmap header size is invalid.\n");
- return false;
+ // All of the headers that reach this point, store color table entries
+ // using 4 bytes per pixel.
+ bytesPerColor = 4;
+ break;
+ case kOS2V1_BmpHeaderType:
+ // The OS2V1 is treated separately because it has a unique format
+ width = (int) get_short(iBuffer.get(), 0);
+ height = (int) get_short(iBuffer.get(), 2);
+ bitsPerPixel = get_short(iBuffer.get(), 6);
+ bytesPerColor = 3;
+ break;
+ case kUnknown_BmpHeaderType:
+ // We'll exit above in this case.
+ SkASSERT(false);
+ return false;
}
// Check for valid dimensions from header
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 342c369..6aa8ed5 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -1032,3 +1032,19 @@
test_info(r, codec.get(), codec->getInfo(), SkCodec::kIncompleteInput, nullptr);
}
+
+DEF_TEST(Codec_InvalidBmp2, r) {
+ // This file reports a header size that crashes when we try to read this
+ // much directly from a file using SkFILEStream.
+ SkString path = GetResourcePath("invalid_images/b33651913.bmp");
+ SkAutoTDelete<SkFILEStream> stream(new SkFILEStream(path.c_str()));
+ if (!stream->isValid()) {
+ ERRORF(r, "no stream");
+ return;
+ }
+
+ SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+ // This file is invalid, but more importantly, we did not crash before
+ // reaching here.
+ REPORTER_ASSERT(r, !codec);
+}
