Merge "Fix potential null read for truncated dex files"

3 files changed, 38 insertions, 3 deletions

diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index c8f661561f..894b33b00a 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -78,6 +78,11 @@ ART_TEST_TARGET_GTEST_MainStripped_DEX := $(basename $(ART_TEST_TARGET_GTEST_Mai
 ART_TEST_HOST_GTEST_MainUncompressed_DEX := $(basename $(ART_TEST_HOST_GTEST_Main_DEX))Uncompressed$(suffix $(ART_TEST_HOST_GTEST_Main_DEX))
 ART_TEST_TARGET_GTEST_MainUncompressed_DEX := $(basename $(ART_TEST_TARGET_GTEST_Main_DEX))Uncompressed$(suffix $(ART_TEST_TARGET_GTEST_Main_DEX))
 
+# Create rules for UncompressedEmpty, a classes.dex that is empty and uncompressed
+# for the dex2oat tests.
+ART_TEST_HOST_GTEST_EmptyUncompressed_DEX := $(basename $(ART_TEST_HOST_GTEST_Main_DEX))EmptyUncompressed$(suffix $(ART_TEST_HOST_GTEST_Main_DEX))
+ART_TEST_TARGET_GTEST_EmptyUncompressed_DEX := $(basename $(ART_TEST_TARGET_GTEST_Main_DEX))EmptyUncompressed$(suffix $(ART_TEST_TARGET_GTEST_Main_DEX))
+
 $(ART_TEST_HOST_GTEST_MainStripped_DEX): $(ART_TEST_HOST_GTEST_Main_DEX)
 	cp $< $@
 	$(call dexpreopt-remove-classes.dex,$@)
@@ -96,6 +101,16 @@ $(ART_TEST_TARGET_GTEST_MainUncompressed_DEX): $(ART_TEST_TARGET_GTEST_Main_DEX)
 	$(call uncompress-dexs, $@)
 	$(call align-package, $@)
 
+$(ART_TEST_HOST_GTEST_EmptyUncompressed_DEX): $(ZIPALIGN)
+	touch $(dir $@)classes.dex
+	zip -j -qD -X -0 $@ $(dir $@)classes.dex
+	rm $(dir $@)classes.dex
+
+$(ART_TEST_TARGET_GTEST_EmptyUncompressed_DEX): $(ZIPALIGN)
+	touch $(dir $@)classes.dex
+	zip -j -qD -X -0 $@ $(dir $@)classes.dex
+	rm $(dir $@)classes.dex
+
 ART_TEST_GTEST_VerifierDeps_SRC := $(abspath $(wildcard $(LOCAL_PATH)/VerifierDeps/*.smali))
 ART_TEST_GTEST_VerifierDepsMulti_SRC := $(abspath $(wildcard $(LOCAL_PATH)/VerifierDepsMulti/*.smali))
 ART_TEST_HOST_GTEST_VerifierDeps_DEX := $(dir $(ART_TEST_HOST_GTEST_Main_DEX))$(subst Main,VerifierDeps,$(basename $(notdir $(ART_TEST_HOST_GTEST_Main_DEX))))$(suffix $(ART_TEST_HOST_GTEST_Main_DEX))
@@ -126,7 +141,7 @@ ART_GTEST_compiler_driver_test_DEX_DEPS := AbstractMethod StaticLeafMethods Prof
 ART_GTEST_dex_cache_test_DEX_DEPS := Main Packages MethodTypes
 ART_GTEST_dex_file_test_DEX_DEPS := GetMethodSignature Main Nested MultiDex
 ART_GTEST_dexlayout_test_DEX_DEPS := ManyMethods
-ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) ManyMethods Statics VerifierDeps MainUncompressed
+ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) ManyMethods Statics VerifierDeps MainUncompressed EmptyUncompressed
 ART_GTEST_dex2oat_image_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) Statics VerifierDeps
 ART_GTEST_exception_test_DEX_DEPS := ExceptionHandle
 ART_GTEST_hiddenapi_test_DEX_DEPS := HiddenApi
@@ -750,6 +765,8 @@ ART_TEST_HOST_GTEST_MainStripped_DEX :=
 ART_TEST_TARGET_GTEST_MainStripped_DEX :=
 ART_TEST_HOST_GTEST_MainUncompressed_DEX :=
 ART_TEST_TARGET_GTEST_MainUncompressed_DEX :=
+ART_TEST_HOST_GTEST_EmptyUncompressed_DEX :=
+ART_TEST_TARGET_GTEST_EmptyUncompressed_DEX :=
 ART_TEST_GTEST_VerifierDeps_SRC :=
 ART_TEST_HOST_GTEST_VerifierDeps_DEX :=
 ART_TEST_TARGET_GTEST_VerifierDeps_DEX :=
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 5614ac6458..17a27f8567 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -1543,4 +1543,20 @@ TEST_F(Dex2oatTest, UncompressedTest) {
                       });
 }
 
+TEST_F(Dex2oatTest, EmptyUncompressedDexTest) {
+  std::string out_dir = GetScratchDir();
+  const std::string base_oat_name = out_dir + "/base.oat";
+  std::string error_msg;
+  int status = GenerateOdexForTestWithStatus(
+      { GetTestDexFileName("MainEmptyUncompressed") },
+      base_oat_name,
+      CompilerFilter::Filter::kQuicken,
+      &error_msg,
+      { },
+      /*use_fd*/ false);
+  // Expect to fail with code 1 and not SIGSEGV or SIGABRT.
+  ASSERT_TRUE(WIFEXITED(status));
+  ASSERT_EQ(WEXITSTATUS(status), 1) << error_msg;
+}
+
 }  // namespace art
diff --git a/runtime/dex/dex_file_loader.cc b/runtime/dex/dex_file_loader.cc
index 0f2758e372..2c75c5b5d9 100644
--- a/runtime/dex/dex_file_loader.cc
+++ b/runtime/dex/dex_file_loader.cc
@@ -319,7 +319,7 @@ std::unique_ptr<DexFile> DexFileLoader::OpenCommon(const uint8_t* base,
     *verify_result = VerifyResult::kVerifyNotAttempted;
   }
   std::unique_ptr<DexFile> dex_file;
-  if (StandardDexFile::IsMagicValid(base)) {
+  if (size >= sizeof(StandardDexFile::Header) && StandardDexFile::IsMagicValid(base)) {
     if (data_size != 0) {
       CHECK_EQ(base, data_base) << "Unsupported for standard dex";
     }
@@ -329,7 +329,7 @@ std::unique_ptr<DexFile> DexFileLoader::OpenCommon(const uint8_t* base,
                                        location_checksum,
                                        oat_dex_file,
                                        container));
-  } else if (CompactDexFile::IsMagicValid(base)) {
+  } else if (size >= sizeof(CompactDexFile::Header) && CompactDexFile::IsMagicValid(base)) {
     if (data_base == nullptr) {
       // TODO: Is there a clean way to support both an explicit data section and reading the one
       // from the header.
@@ -346,6 +346,8 @@ std::unique_ptr<DexFile> DexFileLoader::OpenCommon(const uint8_t* base,
                                       location_checksum,
                                       oat_dex_file,
                                       container));
+  } else {
+    *error_msg = "Invalid or truncated dex file";
   }
   if (dex_file == nullptr) {
     *error_msg = StringPrintf("Failed to open dex file '%s' from memory: %s", location.c_str(),