commit cd16d0aa7e30115323caf040a11b4605f1acfea3
author ganxiaolin <ganxiaolin@xiaomi.com> Mon Jul 18 11:21:44 2016 +0800
committer David Sehr <sehr@google.com> Tue Aug 02 14:46:45 2016 -0700
tree a9a2495f5a50a1a0a514420fdfceb6ab5c6e47fc
parent c984725d0035e1925371757c38fed339b409e525

Return error directly when uncompressed length is 0

If uncompressed length is 0, in function MemMap::MapAnonymous, it will generate a MemMap instance whose base_begin is nullptr.
Then, when OpenMemory, it will create a DexFile instance.
At DexFile::DexFile constructor, header_ is asigned to 0, then header_->string_ids_off_ will cause crash.

Bug: b/28856653
Test: test-art-host-gtest-dex_file_test
Signed-off-by: ganxiaolin <ganxiaolin@xiaomi.com>
Change-Id: Id37f7629f4646cbc385ef054cb83b15be4c59b00

runtime/dex_file.cc

diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 061babd..dff2802 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -338,6 +338,11 @@
     *error_code = ZipOpenErrorCode::kEntryNotFound;
     return nullptr;
   }
+  if (zip_entry->GetUncompressedLength() == 0) {
+    *error_msg = StringPrintf("Dex file '%s' has zero length", location.c_str());
+    *error_code = ZipOpenErrorCode::kDexFileError;
+    return nullptr;
+  }
   std::unique_ptr<MemMap> map(zip_entry->ExtractToMemMap(location.c_str(), entry_name, error_msg));
   if (map.get() == nullptr) {
     *error_msg = StringPrintf("Failed to extract '%s' from '%s': %s", entry_name, location.c_str(),
@@ -435,6 +440,8 @@
                                                    MemMap* mem_map,
                                                    const OatDexFile* oat_dex_file,
                                                    std::string* error_msg) {
+  DCHECK(base != nullptr);
+  DCHECK_NE(size, 0UL);
   CHECK_ALIGNED(base, 4);  // various dex file structures must be word aligned
   std::unique_ptr<DexFile> dex_file(
       new DexFile(base, size, location, location_checksum, mem_map, oat_dex_file));

runtime/dex_file_test.cc

diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index 616c2a0..2704d8a 100644
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -166,6 +166,12 @@
   "uAAAAAYAAAABAAAA0AAAAAEgAAACAAAA8AAAAAEQAAABAAAAHAEAAAIgAAAIAAAAIgEAAAMgAAAC"
   "AAAAcwEAAAAgAAABAAAAfgEAAAAQAAABAAAAjAEAAA==";
 
+static const char kRawDexZeroLength[] =
+  "UEsDBAoAAAAAAOhxAkkAAAAAAAAAAAAAAAALABwAY2xhc3Nlcy5kZXhVVAkAA2QNoVdnDaFXdXgL"
+  "AAEE5AMBAASIEwAAUEsBAh4DCgAAAAAA6HECSQAAAAAAAAAAAAAAAAsAGAAAAAAAAAAAAKCBAAAA"
+  "AGNsYXNzZXMuZGV4VVQFAANkDaFXdXgLAAEE5AMBAASIEwAAUEsFBgAAAAABAAEAUQAAAEUAAAAA"
+  "AA==";
+
 static void DecodeAndWriteDexFile(const char* base64, const char* location) {
   // decode base64
   CHECK(base64 != nullptr);
@@ -254,6 +260,18 @@
   ASSERT_FALSE(DexFile::Open(location, location, kVerifyChecksum, &error_msg, &dex_files));
 }
 
+TEST_F(DexFileTest, ZeroLengthDexRejected) {
+  ScratchFile tmp;
+  const char* location = tmp.GetFilename().c_str();
+  DecodeAndWriteDexFile(kRawDexZeroLength, location);
+
+  ScopedObjectAccess soa(Thread::Current());
+  static constexpr bool kVerifyChecksum = true;
+  std::string error_msg;
+  std::vector<std::unique_ptr<const DexFile>> dex_files;
+  ASSERT_FALSE(DexFile::Open(location, location, kVerifyChecksum, &error_msg, &dex_files));
+}
+
 TEST_F(DexFileTest, GetLocationChecksum) {
   ScopedObjectAccess soa(Thread::Current());
   std::unique_ptr<const DexFile> raw(OpenTestDexFile("Main"));