commit 59399abce0223e74af00193436e39e109d98dacb
author Vladimir Marko <vmarko@google.com> Tue May 03 16:31:52 2016 +0100
committer Vladimir Marko <vmarko@google.com> Thu May 05 15:01:25 2016 +0100
tree fdeba492cd58b2af00b01340f821daa4f68cf216
parent a246510965fc57ec51d1b202649304535adfe9f3

Fix index checks for error strings in DexFileVerifier.

Bug: 28552165
Change-Id: I5b352536b746ec5bd48daa5c693d62d9fbc0b21c

runtime/dex_file_verifier.cc

diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 3df4e98..21ee4bd 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -2358,7 +2358,8 @@
 static std::string GetStringOrError(const uint8_t* const begin,
                                     const DexFile::Header* const header,
                                     uint32_t string_idx) {
-  if (header->string_ids_size_ < string_idx) {
+  // The `string_idx` is not guaranteed to be valid yet.
+  if (header->string_ids_size_ <= string_idx) {
     return "(error)";
   }
 
@@ -2375,9 +2376,11 @@
 static std::string GetClassOrError(const uint8_t* const begin,
                                    const DexFile::Header* const header,
                                    uint32_t class_idx) {
-  if (header->type_ids_size_ < class_idx) {
-    return "(error)";
-  }
+  // The `class_idx` is either `FieldId::class_idx_` or `MethodId::class_idx_` and
+  // it has already been checked in `DexFileVerifier::CheckClassDataItemField()`
+  // or `DexFileVerifier::CheckClassDataItemMethod()`, respectively, to match
+  // a valid defining class.
+  CHECK_LT(class_idx, header->type_ids_size_);
 
   const DexFile::TypeId* type_id =
       reinterpret_cast<const DexFile::TypeId*>(begin + header->type_ids_off_) + class_idx;
@@ -2390,9 +2393,8 @@
 static std::string GetFieldDescriptionOrError(const uint8_t* const begin,
                                               const DexFile::Header* const header,
                                               uint32_t idx) {
-  if (header->field_ids_size_ < idx) {
-    return "(error)";
-  }
+  // The `idx` has already been checked in `DexFileVerifier::CheckClassDataItemField()`.
+  CHECK_LT(idx, header->field_ids_size_);
 
   const DexFile::FieldId* field_id =
       reinterpret_cast<const DexFile::FieldId*>(begin + header->field_ids_off_) + idx;
@@ -2408,9 +2410,8 @@
 static std::string GetMethodDescriptionOrError(const uint8_t* const begin,
                                                const DexFile::Header* const header,
                                                uint32_t idx) {
-  if (header->method_ids_size_ < idx) {
-    return "(error)";
-  }
+  // The `idx` has already been checked in `DexFileVerifier::CheckClassDataItemMethod()`.
+  CHECK_LT(idx, header->method_ids_size_);
 
   const DexFile::MethodId* method_id =
       reinterpret_cast<const DexFile::MethodId*>(begin + header->method_ids_off_) + idx;