Merge cherrypicks of [2420017, 2420032, 2420103, 2420142, 2420125, 2420126, 2420086, 2420018, 2420104, 2420162, 2420163, 2420164, 2420143, 2420019, 2420034, 2420055, 2420127, 2420128, 2420129, 2420020, 2420166, 2420167, 2420058, 2420131, 2420202, 2420108, 2420146, 2420109, 2420185, 2420111, 2420187, 2420113, 2420114, 2420059, 2420115] into nyc-mr2-security-b-release
Change-Id: I81ea2d20972346b483f979b83244c0bd34b7d365
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);
+}