commit 14e7badae4f118ecd8481913c927316737fd21fa
author Mathieu Chartier <mathieuc@google.com> Thu Mar 22 14:33:20 2018 -0700
committer Mathieu Chartier <mathieuc@google.com> Thu Mar 22 16:03:05 2018 -0700
tree d2bc49f6d9545d33e90fc33a95c6246d041be9fc
parent 7af7d8dccc6d96d5d85564a58f5b18169a2f969b

Verify dex files in zip and from files are not CompactDex

Since there is no CompactDex verifier, dex2oat may crash for invalid
CompactDex files in the APK or directly as files. Disallow opening
these to prevent crashes.

Bug: 75967391
Bug: 63756964
Test: test-art-host-gtest

Change-Id: Ifc86f7bc2a478201473aad6481bf1e3435a910ae

dex2oat/dex2oat_test.cc

diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 5e9782a..0cd39ac 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -2046,4 +2046,51 @@
   ASSERT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) != 0) << status << " " << output_;
 }
 
+// Test that dex2oat with a CompactDex file in the APK fails.
+TEST_F(Dex2oatTest, CompactDexInZip) {
+  CompactDexFile::Header header = {};
+  CompactDexFile::WriteMagic(header.magic_);
+  CompactDexFile::WriteCurrentVersion(header.magic_);
+  header.file_size_ = sizeof(CompactDexFile::Header);
+  header.data_off_ = 10 * MB;
+  header.map_off_ = 10 * MB;
+  header.class_defs_off_ = 10 * MB;
+  header.class_defs_size_ = 10000;
+  // Create a zip containing the invalid dex.
+  ScratchFile invalid_dex_zip;
+  {
+    FILE* file = fdopen(invalid_dex_zip.GetFd(), "w+b");
+    ZipWriter writer(file);
+    writer.StartEntry("classes.dex", ZipWriter::kCompress);
+    ASSERT_GE(writer.WriteBytes(&header, sizeof(header)), 0);
+    writer.FinishEntry();
+    writer.Finish();
+    ASSERT_EQ(invalid_dex_zip.GetFile()->Flush(), 0);
+  }
+  // Create the dex file directly.
+  ScratchFile invalid_dex;
+  {
+    ASSERT_GE(invalid_dex.GetFile()->WriteFully(&header, sizeof(header)), 0);
+    ASSERT_EQ(invalid_dex.GetFile()->Flush(), 0);
+  }
+  std::string error_msg;
+  int status = 0u;
+
+  status = GenerateOdexForTestWithStatus(
+      { invalid_dex_zip.GetFilename() },
+      GetOdexDir() + "/output_apk.odex",
+      CompilerFilter::kQuicken,
+      &error_msg,
+      { "--compact-dex-level=fast" });
+  ASSERT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) != 0) << status << " " << output_;
+
+  status = GenerateOdexForTestWithStatus(
+      { invalid_dex.GetFilename() },
+      GetOdexDir() + "/output.odex",
+      CompilerFilter::kQuicken,
+      &error_msg,
+      { "--compact-dex-level=fast" });
+  ASSERT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) != 0) << status << " " << output_;
+}
+
 }  // namespace art

runtime/dex/art_dex_file_loader.cc

diff --git a/runtime/dex/art_dex_file_loader.cc b/runtime/dex/art_dex_file_loader.cc
index c456764..9802c69 100644
--- a/runtime/dex/art_dex_file_loader.cc
+++ b/runtime/dex/art_dex_file_loader.cc
@@ -205,6 +205,12 @@
                                                  error_msg,
                                                  std::make_unique<MemMapContainer>(std::move(map)),
                                                  /*verify_result*/ nullptr);
+  // Opening CompactDex is only supported from vdex files.
+  if (dex_file != nullptr && dex_file->IsCompactDexFile()) {
+    *error_msg = StringPrintf("Opening CompactDex file '%s' is only supported from vdex files",
+                              location.c_str());
+    return nullptr;
+  }
   return dex_file;
 }
 
@@ -329,6 +335,12 @@
                                                  std::make_unique<MemMapContainer>(std::move(map)),
                                                  /*verify_result*/ nullptr);
 
+  // Opening CompactDex is only supported from vdex files.
+  if (dex_file != nullptr && dex_file->IsCompactDexFile()) {
+    *error_msg = StringPrintf("Opening CompactDex file '%s' is only supported from vdex files",
+                              location.c_str());
+    return nullptr;
+  }
   return dex_file;
 }
 
@@ -397,6 +409,11 @@
                                                  error_msg,
                                                  std::make_unique<MemMapContainer>(std::move(map)),
                                                  &verify_result);
+  if (dex_file != nullptr && dex_file->IsCompactDexFile()) {
+    *error_msg = StringPrintf("Opening CompactDex file '%s' is only supported from vdex files",
+                              location.c_str());
+    return nullptr;
+  }
   if (dex_file == nullptr) {
     if (verify_result == VerifyResult::kVerifyNotAttempted) {
       *error_code = ZipOpenErrorCode::kDexFileError;