diff options
author | 2023-05-30 16:17:39 +0100 | |
---|---|---|
committer | 2023-06-02 10:34:48 +0000 | |
commit | ffa7ca3c4304a7c9c1e0244382dac662dd14f124 (patch) | |
tree | d5e20c0299b39c9d8f3305aa0dc3703cc13a2c1a | |
parent | d36df665cd24c16ce08ca280cd84f49a46af2026 (diff) |
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
-rw-r--r-- | libdexfile/dex/class_accessor.h | 3 | ||||
-rw-r--r-- | libdexfile/dex/dex_file_loader_test.cc | 18 | ||||
-rw-r--r-- | libdexfile/dex/dex_file_verifier.cc | 168 |
3 files changed, 114 insertions, 75 deletions
diff --git a/libdexfile/dex/class_accessor.h b/libdexfile/dex/class_accessor.h index a3ee2bd386..1061ff69ec 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 @@ class ClassAccessor { 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 812415b065..0ae59d3de6 100644 --- a/libdexfile/dex/dex_file_loader_test.cc +++ b/libdexfile/dex/dex_file_loader_test.cc @@ -243,6 +243,20 @@ static const char kRawDexCodeItemOOB[] = "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 @@ TEST_F(DexFileLoaderTest, CodeItemOutOfBounds) { 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 e16f72bb38..fac703fe84 100644 --- a/libdexfile/dex/dex_file_verifier.cc +++ b/libdexfile/dex/dex_file_verifier.cc @@ -314,6 +314,7 @@ class DexFileVerifier { 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 @@ bool DexFileVerifier::CheckIntraHiddenapiClassData() { 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 @@ uint32_t DexFileVerifier::FindFirstAnnotationsDirectoryDefiner(const uint8_t* pt 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 @@ bool DexFileVerifier::CheckInterSectionIterate(size_t offset, 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; |