Merge cherrypicks of [2435782, 2435615, 2435577, 2435653, 2435616, 2435708, 2435617, 2435558, 2435784, 2435709, 2435631, 2435559, 2435560, 2435618, 2435801, 2435674, 2435710, 2435746, 2435579, 2435747, 2435711, 2435785, 2435786, 2435787, 2435713, 2435804, 2435822, 2435842, 2435753, 2435965, 2436024, 2435885] into nyc-mr1-security-e-release
Change-Id: I797ec9bfc9847b3542b79628103b22440b3a4aab
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);
+}