ART: Check alignment of section offsets

Make sure the sections mentioned in the header are aligned according
to the Dalvik File Format specification.

Ensure the same for annotations.

Bug: 27275385
Bug: https://code.google.com/p/android/issues/detail?id=201384
Change-Id: Ifdd98377f8468e78c1c2198223ad58cab302dd37
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index ddf2749..9c9b8c5 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -230,7 +230,10 @@
   return true;
 }
 
-bool DexFileVerifier::CheckValidOffsetAndSize(uint32_t offset, uint32_t size, const char* label) {
+bool DexFileVerifier::CheckValidOffsetAndSize(uint32_t offset,
+                                              uint32_t size,
+                                              size_t alignment,
+                                              const char* label) {
   if (size == 0) {
     if (offset != 0) {
       ErrorStringPrintf("Offset(%d) should be zero when size is zero for %s.", offset, label);
@@ -241,6 +244,10 @@
     ErrorStringPrintf("Offset(%d) should be within file size(%zu) for %s.", offset, size_, label);
     return false;
   }
+  if (alignment != 0 && !IsAlignedParam(offset, alignment)) {
+    ErrorStringPrintf("Offset(%d) should be aligned by %zu for %s.", offset, alignment, label);
+    return false;
+  }
   return true;
 }
 
@@ -275,16 +282,43 @@
 
   // Check that all offsets are inside the file.
   bool result =
-      CheckValidOffsetAndSize(header_->link_off_, header_->link_size_, "link") &&
-      CheckValidOffsetAndSize(header_->map_off_, header_->map_off_, "map") &&
-      CheckValidOffsetAndSize(header_->string_ids_off_, header_->string_ids_size_, "string-ids") &&
-      CheckValidOffsetAndSize(header_->type_ids_off_, header_->type_ids_size_, "type-ids") &&
-      CheckValidOffsetAndSize(header_->proto_ids_off_, header_->proto_ids_size_, "proto-ids") &&
-      CheckValidOffsetAndSize(header_->field_ids_off_, header_->field_ids_size_, "field-ids") &&
-      CheckValidOffsetAndSize(header_->method_ids_off_, header_->method_ids_size_, "method-ids") &&
-      CheckValidOffsetAndSize(header_->class_defs_off_, header_->class_defs_size_, "class-defs") &&
-      CheckValidOffsetAndSize(header_->data_off_, header_->data_size_, "data");
-
+      CheckValidOffsetAndSize(header_->link_off_,
+                              header_->link_size_,
+                              0 /* unaligned */,
+                              "link") &&
+      CheckValidOffsetAndSize(header_->map_off_,
+                              header_->map_off_,
+                              4,
+                              "map") &&
+      CheckValidOffsetAndSize(header_->string_ids_off_,
+                              header_->string_ids_size_,
+                              4,
+                              "string-ids") &&
+      CheckValidOffsetAndSize(header_->type_ids_off_,
+                              header_->type_ids_size_,
+                              4,
+                              "type-ids") &&
+      CheckValidOffsetAndSize(header_->proto_ids_off_,
+                              header_->proto_ids_size_,
+                              4,
+                              "proto-ids") &&
+      CheckValidOffsetAndSize(header_->field_ids_off_,
+                              header_->field_ids_size_,
+                              4,
+                              "field-ids") &&
+      CheckValidOffsetAndSize(header_->method_ids_off_,
+                              header_->method_ids_size_,
+                              4,
+                              "method-ids") &&
+      CheckValidOffsetAndSize(header_->class_defs_off_,
+                              header_->class_defs_size_,
+                              4,
+                              "class-defs") &&
+      CheckValidOffsetAndSize(header_->data_off_,
+                              header_->data_size_,
+                              0,  // Unaligned, spec doesn't talk about it, even though size
+                                  // is supposed to be a multiple of 4.
+                              "data");
   return result;
 }
 
@@ -1965,6 +1999,11 @@
 
   // Check that references in annotations_directory_item are to right class.
   if (item->annotations_off_ != 0) {
+    // annotations_off_ is supposed to be aligned by 4.
+    if (!IsAlignedParam(item->annotations_off_, 4)) {
+      ErrorStringPrintf("Invalid annotations_off_, not aligned by 4");
+      return false;
+    }
     const uint8_t* data = begin_ + item->annotations_off_;
     bool success;
     uint16_t annotations_definer = FindFirstAnnotationsDirectoryDefiner(data, &success);
diff --git a/runtime/dex_file_verifier.h b/runtime/dex_file_verifier.h
index ddfeea2..be0e6d8 100644
--- a/runtime/dex_file_verifier.h
+++ b/runtime/dex_file_verifier.h
@@ -48,7 +48,7 @@
   bool CheckList(size_t element_size, const char* label, const uint8_t* *ptr);
   // Checks whether the offset is zero (when size is zero) or that the offset falls within the area
   // claimed by the file.
-  bool CheckValidOffsetAndSize(uint32_t offset, uint32_t size, const char* label);
+  bool CheckValidOffsetAndSize(uint32_t offset, uint32_t size, size_t alignment, const char* label);
   bool CheckIndex(uint32_t field, uint32_t limit, const char* label);
 
   bool CheckHeader();
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 558a6ed..44cf2ee 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -1253,4 +1253,63 @@
       "DBG_START_LOCAL type_idx");
 }
 
+TEST_F(DexFileVerifierTest, SectionAlignment) {
+  {
+    // The input dex file should be good before modification. Any file is fine, as long as it
+    // uses all sections.
+    ScratchFile tmp;
+    std::string error_msg;
+    std::unique_ptr<const DexFile> raw(OpenDexFileBase64(kGoodTestDex,
+                                                         tmp.GetFilename().c_str(),
+                                                         &error_msg));
+    ASSERT_TRUE(raw.get() != nullptr) << error_msg;
+  }
+
+  // Modify all section offsets to be unaligned.
+  constexpr size_t kSections = 7;
+  for (size_t i = 0; i < kSections; ++i) {
+    VerifyModification(
+        kGoodTestDex,
+        "section_align",
+        [&](DexFile* dex_file) {
+          DexFile::Header* header = const_cast<DexFile::Header*>(
+              reinterpret_cast<const DexFile::Header*>(dex_file->Begin()));
+          uint32_t* off_ptr;
+          switch (i) {
+            case 0:
+              off_ptr = &header->map_off_;
+              break;
+            case 1:
+              off_ptr = &header->string_ids_off_;
+              break;
+            case 2:
+              off_ptr = &header->type_ids_off_;
+              break;
+            case 3:
+              off_ptr = &header->proto_ids_off_;
+              break;
+            case 4:
+              off_ptr = &header->field_ids_off_;
+              break;
+            case 5:
+              off_ptr = &header->method_ids_off_;
+              break;
+            case 6:
+              off_ptr = &header->class_defs_off_;
+              break;
+
+            static_assert(kSections == 7, "kSections is wrong");
+            default:
+              LOG(FATAL) << "Unexpected section";
+              UNREACHABLE();
+          }
+          ASSERT_TRUE(off_ptr != nullptr);
+          ASSERT_NE(*off_ptr, 0U) << i;  // Should already contain a value (in use).
+          (*off_ptr)++;                  // Add one, which should misalign it (all the sections
+                                         // above are aligned by 4).
+        },
+        "should be aligned by 4 for");
+  }
+}
+
 }  // namespace art