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;