Fix potential null read for truncated dex files
In the case where a dex file is truncated, don't attempt to read the
header.
Added regression test.
Bug: 63756964
Bug: 72826975
Test: mm test-art-host-gtest-dex2oat_test -j64
Test: mm test-art-target-gtest-dex2oat_test -j64
Change-Id: I9bd6bb445ef2eb1c961044f43ac71f04ef8c04a5
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index c8f6615..894b33b 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -78,6 +78,11 @@
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 @@
$(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_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_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 5614ac6..17a27f8 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -1543,4 +1543,20 @@
});
}
+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 0f2758e..2c75c5b 100644
--- a/runtime/dex/dex_file_loader.cc
+++ b/runtime/dex/dex_file_loader.cc
@@ -319,7 +319,7 @@
*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 @@
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 @@
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(),