commit a4cd6779fc5fc73d84d5ca269b89a33064267cce
author Jeff Hao <jeffhao@google.com> Thu Apr 13 14:36:29 2017 -0700
committer Jeff Hao <jeffhao@google.com> Thu Apr 13 14:43:13 2017 -0700
tree fbfea67a5f6e284ec7b8dc3a2022d6f03579666f
parent ce121a5fe3e537872af077c801443be9a525859b

Add check that map offset is sane to DexFile object init.

The DexFile constructor calls InitializeSectionsFromMapList, which
uses the header's map_offset before any checks are done on the file.

Bug: 37235346
Test: mm test-art-host-gtest-dex_file_test
Change-Id: I4fb8fcb57f9ef7e0182965b7ce663424b953abcb

runtime/dex_file.cc

diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 85100ae..688f95b 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -590,6 +590,10 @@
 
 void DexFile::InitializeSectionsFromMapList() {
   const MapList* map_list = reinterpret_cast<const MapList*>(begin_ + header_->map_off_);
+  if (header_->map_off_ == 0 || header_->map_off_ > size_) {
+    // Bad offset. The dex file verifier runs after this method and will reject the file.
+    return;
+  }
   const size_t count = map_list->size_;
 
   size_t map_limit = header_->map_off_ + count * sizeof(MapItem);

runtime/dex_file_test.cc

diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index 9131715..3f7461c 100644
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -161,6 +161,16 @@
   "ACACAAALABgAAAAAAAAAAACgge4CAABjbGFzc2VzLmRleFVUBQADAWPlV3V4CwABBOQDAQAEiBMA"
   "AFBLBQYAAAAAAwADAPUAAABkBAAAAAA=";
 
+static const char kRawDexBadMapOffset[] =
+  "ZGV4CjAzNQAZKGSz85r+tXJ1I24FYi+FpQtWbXtelAmoAQAAcAAAAHhWNBIAAAAAAAAAAEAwIBAF"
+  "AAAAcAAAAAMAAACEAAAAAQAAAJAAAAAAAAAAAAAAAAIAAACcAAAAAQAAAKwAAADcAAAAzAAAAOQA"
+  "AADsAAAA9AAAAPkAAAANAQAAAgAAAAMAAAAEAAAABAAAAAIAAAAAAAAAAAAAAAAAAAABAAAAAAAA"
+  "AAAAAAABAAAAAQAAAAAAAAABAAAAAAAAABUBAAAAAAAAAQABAAEAAAAQAQAABAAAAHAQAQAAAA4A"
+  "Bjxpbml0PgAGQS5qYXZhAANMQTsAEkxqYXZhL2xhbmcvT2JqZWN0OwABVgABAAcOAAAAAQAAgYAE"
+  "zAEACwAAAAAAAAABAAAAAAAAAAEAAAAFAAAAcAAAAAIAAAADAAAAhAAAAAMAAAABAAAAkAAAAAUA"
+  "AAACAAAAnAAAAAYAAAABAAAArAAAAAEgAAABAAAAzAAAAAIgAAAFAAAA5AAAAAMgAAABAAAAEAEA"
+  "AAAgAAABAAAAFQEAAAAQAAABAAAAIAEAAA==";
+
 static void DecodeAndWriteDexFile(const char* base64, const char* location) {
   // decode base64
   CHECK(base64 != nullptr);
@@ -212,7 +222,8 @@
 
 static std::unique_ptr<const DexFile> OpenDexFileInMemoryBase64(const char* base64,
                                                                 const char* location,
-                                                                uint32_t location_checksum) {
+                                                                uint32_t location_checksum,
+                                                                bool expect_success) {
   CHECK(base64 != nullptr);
   std::vector<uint8_t> dex_bytes = DecodeBase64Vec(base64);
   CHECK_NE(dex_bytes.size(), 0u);
@@ -232,7 +243,11 @@
                                                         /* verify */ true,
                                                         /* verify_checksum */ true,
                                                         &error_message));
-  CHECK(dex_file != nullptr) << error_message;
+  if (expect_success) {
+    CHECK(dex_file != nullptr) << error_message;
+  } else {
+    CHECK(dex_file == nullptr) << "Expected dex file open to fail.";
+  }
   return dex_file;
 }
 
@@ -282,7 +297,7 @@
 TEST_F(DexFileTest, HeaderInMemory) {
   ScratchFile tmp;
   std::unique_ptr<const DexFile> raw =
-      OpenDexFileInMemoryBase64(kRawDex, tmp.GetFilename().c_str(), 0x00d87910U);
+      OpenDexFileInMemoryBase64(kRawDex, tmp.GetFilename().c_str(), 0x00d87910U, true);
   ValidateDexFileHeader(std::move(raw));
 }
 
@@ -569,4 +584,11 @@
   EXPECT_EQ(dex_files.size(), 3u);
 }
 
+TEST_F(DexFileTest, OpenDexBadMapOffset) {
+  ScratchFile tmp;
+  std::unique_ptr<const DexFile> raw =
+      OpenDexFileInMemoryBase64(kRawDexBadMapOffset, tmp.GetFilename().c_str(), 0xb3642819U, false);
+  EXPECT_EQ(raw, nullptr);
+}
+
 }  // namespace art