Separate hiddenapi_class_data_item checks in intra and inter

Some of those checks relied on class_data_item being verified
first, so we can move them to the `inter` section.

Bug: 283636595
Fixes: 283636595
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Test: m test-art-host-gtest-art_libdexfile_tests64
Change-Id: I33c9226d0e25a7fdd1725754c52bfebbefa98991
diff --git a/libdexfile/dex/class_accessor.h b/libdexfile/dex/class_accessor.h
index a3ee2bd..1061ff6 100644
--- a/libdexfile/dex/class_accessor.h
+++ b/libdexfile/dex/class_accessor.h
@@ -18,6 +18,7 @@
 #define ART_LIBDEXFILE_DEX_CLASS_ACCESSOR_H_
 
 #include "code_item_accessors.h"
+#include "dex/dex_file.h"
 #include "dex_file_types.h"
 #include "invoke_type.h"
 #include "modifiers.h"
@@ -279,7 +280,7 @@
 
   ClassAccessor(const DexFile& dex_file,
                 const uint8_t* class_data,
-                uint32_t class_def_index = dex::kDexNoIndex,
+                uint32_t class_def_index = DexFile::kDexNoIndex32,
                 bool parse_hiddenapi_class_data = false);
 
   // Return the code item for a method.
diff --git a/libdexfile/dex/dex_file_loader_test.cc b/libdexfile/dex/dex_file_loader_test.cc
index 812415b..0ae59d3 100644
--- a/libdexfile/dex/dex_file_loader_test.cc
+++ b/libdexfile/dex/dex_file_loader_test.cc
@@ -243,6 +243,20 @@
     "BgAAAAEAAADYAAAAASAAAAIAAAD4AAAAARAAAAEAAAAkAQAAAiAAAAoAAAAqAQAA"
     "AyAAAAIAAACHAQAAACAAAAEAAACSAQAAABAAAAEAAACgAQAA";
 
+static const char kHiddenAPIClassDataBadOffset[] =
+    "ZGV4CjAzNQA+Lt8iLnmGPLR973zkwLwGomvp/qfZL080AgAAcAAAAHhWNBIAAAAA"
+    "AAAAAKABAAAKAAAAcAAAAAQAAACYAAAAAgAAAKgAAAAAAAAAAAAAAAMAAADAAAAA"
+    "AQAAANgAAAA8AfoA+AAAACoBAAAyAQAAOgEAAE4BAABZAQAAXAEAAGABAAB1AQAA"
+    "ewEAAIEBAAABAAAAAgAAAAQAAAAGAAAABAAAAAIAAAAAAAAABQAAAAIAAABHAQBP"
+    "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/"
+    "//////8DRwAAAAAAAAAAAAAAAAAIAAAAAAAAAPIAAAAIAAAAAAAAAAAAAAAAAAAA"
+    "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+    "ACAAAAACAAAABAAAAJgAAAADAAAAAgAAAKgAAAAFAAAAAwAAAMAAAAAGAAAAAQAA"
+    "AAR0aGlzABEAAA4AEwEIBw4AAAACAAAlgPsH/gEJkAIMAAAAAAAAAAEAAAAAAAAA"
+    "AQAAAAoAAABwAAAAAgAAAAQAAACYAAAAAwAAAAIAAACoAAAABQAAAAMAAADAAAAA"
+    "BgAAAAEAAADYAAAABwAAAAIAAAD4AAAAAPAAAAAAAQAEAQAAAiAAAAoAAAAqAQAA"
+    "AyAAAC0AAACHAQAAACAAAAEAAACSAQAAABAAAAEAAACgAQAA";
+
 static void DecodeDexFile(const char* base64, std::vector<uint8_t>* dex_bytes) {
   // decode base64
   CHECK(base64 != nullptr);
@@ -535,4 +549,8 @@
   OpenAndVerify(kRawDexCodeItemOOB, /*expected_success=*/false);
 }
 
+TEST_F(DexFileLoaderTest, HiddenAPIClassDataBadOffset) {
+  OpenAndVerify(kHiddenAPIClassDataBadOffset, /*expected_success=*/false);
+}
+
 }  // namespace art
diff --git a/libdexfile/dex/dex_file_verifier.cc b/libdexfile/dex/dex_file_verifier.cc
index e16f72b..fac703f 100644
--- a/libdexfile/dex/dex_file_verifier.cc
+++ b/libdexfile/dex/dex_file_verifier.cc
@@ -314,6 +314,7 @@
   uint32_t FindFirstClassDataDefiner(const ClassAccessor& accessor);
   uint32_t FindFirstAnnotationsDirectoryDefiner(const uint8_t* ptr);
 
+  bool CheckInterHiddenapiClassData();
   bool CheckInterStringIdItem();
   bool CheckInterTypeIdItem();
   bool CheckInterProtoIdItem();
@@ -1870,80 +1871,10 @@
     return false;
   }
 
+  // The rest of the section depends on the class_data_item being verified first. We will finalize
+  // verifying the hiddenapi_class_data_item in CheckInterHiddenapiClassData.
   const uint8_t* data_end = ptr_ + item->size_;
-  ptr_ += header_size;
-
-  // Check offsets for each class def.
-  for (uint32_t i = 0; i < dex_file_->NumClassDefs(); ++i) {
-    const dex::ClassDef& class_def = dex_file_->GetClassDef(i);
-    const uint8_t* class_data = dex_file_->GetClassData(class_def);
-    uint32_t offset = item->flags_offset_[i];
-
-    if (offset == 0) {
-      continue;
-    }
-
-    // Check that class defs with no class data do not have any hiddenapi class data.
-    if (class_data == nullptr) {
-      ErrorStringPrintf(
-          "Hiddenapi class data offset not zero for class def %u with no class data", i);
-      return false;
-    }
-
-    // Check that the offset is within the section.
-    if (offset > item->size_) {
-      ErrorStringPrintf(
-          "Hiddenapi class data offset out of section bounds (%u > %u) for class def %u",
-          offset, item->size_, i);
-      return false;
-    }
-
-    // Check that the offset matches current pointer position. We do not allow
-    // offsets into already parsed data, or gaps between class def data.
-    uint32_t ptr_offset = ptr_ - reinterpret_cast<const uint8_t*>(item);
-    if (offset != ptr_offset) {
-      ErrorStringPrintf(
-          "Hiddenapi class data unexpected offset (%u != %u) for class def %u",
-          offset, ptr_offset, i);
-      return false;
-    }
-
-    // Parse a uleb128 value for each field and method of this class.
-    bool failure = false;
-    auto fn_member = [&](const ClassAccessor::BaseItem& member, const char* member_type) {
-      if (failure) {
-        return;
-      }
-      uint32_t decoded_flags;
-      if (!DecodeUnsignedLeb128Checked(&ptr_, data_end, &decoded_flags)) {
-        ErrorStringPrintf("Hiddenapi class data value out of bounds (%p > %p) for %s %i",
-                          ptr_, data_end, member_type, member.GetIndex());
-        failure = true;
-        return;
-      }
-      if (!hiddenapi::ApiList(decoded_flags).IsValid()) {
-        ErrorStringPrintf("Hiddenapi class data flags invalid (%u) for %s %i",
-                          decoded_flags, member_type, member.GetIndex());
-        failure = true;
-        return;
-      }
-    };
-    auto fn_field = [&](const ClassAccessor::Field& field) { fn_member(field, "field"); };
-    auto fn_method = [&](const ClassAccessor::Method& method) { fn_member(method, "method"); };
-    ClassAccessor accessor(*dex_file_, class_data);
-    accessor.VisitFieldsAndMethods(fn_field, fn_field, fn_method, fn_method);
-    if (failure) {
-      return false;
-    }
-  }
-
-  if (ptr_ != data_end) {
-    ErrorStringPrintf("Hiddenapi class data wrong reported size (%u != %u)",
-                       static_cast<uint32_t>(ptr_ - reinterpret_cast<const uint8_t*>(item)),
-                       item->size_);
-    return false;
-  }
-
+  ptr_ = data_end;
   return true;
 }
 
@@ -2466,6 +2397,90 @@
   return kDexNoIndex;
 }
 
+bool DexFileVerifier::CheckInterHiddenapiClassData() {
+  const dex::HiddenapiClassData* item = reinterpret_cast<const dex::HiddenapiClassData*>(ptr_);
+
+  // Move pointer after the header. This data has been verified in CheckIntraHiddenapiClassData.
+  uint32_t num_header_elems = dex_file_->NumClassDefs() + 1;
+  uint32_t elem_size = sizeof(uint32_t);
+  uint32_t header_size = num_header_elems * elem_size;
+  const uint8_t* data_end = ptr_ + item->size_;
+  ptr_ += header_size;
+
+  // Check offsets for each class def.
+  for (uint32_t i = 0; i < dex_file_->NumClassDefs(); ++i) {
+    const dex::ClassDef& class_def = dex_file_->GetClassDef(i);
+    const uint8_t* class_data = dex_file_->GetClassData(class_def);
+    uint32_t offset = item->flags_offset_[i];
+
+    if (offset == 0) {
+      continue;
+    }
+
+    // Check that class defs with no class data do not have any hiddenapi class data.
+    if (class_data == nullptr) {
+      ErrorStringPrintf(
+          "Hiddenapi class data offset not zero for class def %u with no class data", i);
+      return false;
+    }
+
+    // Check that the offset is within the section.
+    if (offset > item->size_) {
+      ErrorStringPrintf(
+          "Hiddenapi class data offset out of section bounds (%u > %u) for class def %u",
+          offset, item->size_, i);
+      return false;
+    }
+
+    // Check that the offset matches current pointer position. We do not allow
+    // offsets into already parsed data, or gaps between class def data.
+    uint32_t ptr_offset = ptr_ - reinterpret_cast<const uint8_t*>(item);
+    if (offset != ptr_offset) {
+      ErrorStringPrintf(
+          "Hiddenapi class data unexpected offset (%u != %u) for class def %u",
+          offset, ptr_offset, i);
+      return false;
+    }
+
+    // Parse a uleb128 value for each field and method of this class.
+    bool failure = false;
+    auto fn_member = [&](const ClassAccessor::BaseItem& member, const char* member_type) {
+      if (failure) {
+        return;
+      }
+      uint32_t decoded_flags;
+      if (!DecodeUnsignedLeb128Checked(&ptr_, data_end, &decoded_flags)) {
+        ErrorStringPrintf("Hiddenapi class data value out of bounds (%p > %p) for %s %i",
+                          ptr_, data_end, member_type, member.GetIndex());
+        failure = true;
+        return;
+      }
+      if (!hiddenapi::ApiList(decoded_flags).IsValid()) {
+        ErrorStringPrintf("Hiddenapi class data flags invalid (%u) for %s %i",
+                          decoded_flags, member_type, member.GetIndex());
+        failure = true;
+        return;
+      }
+    };
+    auto fn_field = [&](const ClassAccessor::Field& field) { fn_member(field, "field"); };
+    auto fn_method = [&](const ClassAccessor::Method& method) { fn_member(method, "method"); };
+    ClassAccessor accessor(*dex_file_, class_data);
+    accessor.VisitFieldsAndMethods(fn_field, fn_field, fn_method, fn_method);
+    if (failure) {
+      return false;
+    }
+  }
+
+  if (ptr_ != data_end) {
+    ErrorStringPrintf("Hiddenapi class data wrong reported size (%u != %u)",
+                       static_cast<uint32_t>(ptr_ - reinterpret_cast<const uint8_t*>(item)),
+                       item->size_);
+    return false;
+  }
+
+  return true;
+}
+
 bool DexFileVerifier::CheckInterStringIdItem() {
   const dex::StringId* item = reinterpret_cast<const dex::StringId*>(ptr_);
 
@@ -3132,8 +3147,13 @@
       case DexFile::kDexTypeDebugInfoItem:
       case DexFile::kDexTypeAnnotationItem:
       case DexFile::kDexTypeEncodedArrayItem:
-      case DexFile::kDexTypeHiddenapiClassData:
         break;
+      case DexFile::kDexTypeHiddenapiClassData: {
+        if (!CheckIntraHiddenapiClassData()) {
+          return false;
+        }
+        break;
+      }
       case DexFile::kDexTypeStringIdItem: {
         if (!CheckInterStringIdItem()) {
           return false;