diff options
| -rw-r--r-- | libdexfile/dex/class_accessor-inl.h | 9 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor.h | 18 | ||||
| -rw-r--r-- | libdexfile/dex/dex_file.h | 1 | ||||
| -rw-r--r-- | libdexfile/dex/dex_file_verifier.cc | 189 | ||||
| -rw-r--r-- | libdexfile/dex/dex_file_verifier.h | 15 |
5 files changed, 150 insertions, 82 deletions
diff --git a/libdexfile/dex/class_accessor-inl.h b/libdexfile/dex/class_accessor-inl.h index dd91438ff7..21db2cf2be 100644 --- a/libdexfile/dex/class_accessor-inl.h +++ b/libdexfile/dex/class_accessor-inl.h @@ -32,9 +32,16 @@ inline ClassAccessor::ClassAccessor(const DexFile& dex_file, const DexFile::Clas : ClassAccessor(dex_file, dex_file.GetIndexForClassDef(class_def)) {} inline ClassAccessor::ClassAccessor(const DexFile& dex_file, uint32_t class_def_index) + : ClassAccessor(dex_file, + dex_file.GetClassData(dex_file.GetClassDef(class_def_index)), + class_def_index) {} + +inline ClassAccessor::ClassAccessor(const DexFile& dex_file, + const uint8_t* class_data, + uint32_t class_def_index) : dex_file_(dex_file), class_def_index_(class_def_index), - ptr_pos_(dex_file.GetClassData(dex_file.GetClassDef(class_def_index))), + ptr_pos_(class_data), num_static_fields_(ptr_pos_ != nullptr ? DecodeUnsignedLeb128(&ptr_pos_) : 0u), num_instance_fields_(ptr_pos_ != nullptr ? DecodeUnsignedLeb128(&ptr_pos_) : 0u), num_direct_methods_(ptr_pos_ != nullptr ? DecodeUnsignedLeb128(&ptr_pos_) : 0u), diff --git a/libdexfile/dex/class_accessor.h b/libdexfile/dex/class_accessor.h index 5579be21e5..896fcadb10 100644 --- a/libdexfile/dex/class_accessor.h +++ b/libdexfile/dex/class_accessor.h @@ -133,6 +133,7 @@ class ClassAccessor { uint32_t code_off_ = 0u; friend class ClassAccessor; + friend class DexFileVerifier; }; // A decoded version of the field of a class_data_item. @@ -159,6 +160,7 @@ class ClassAccessor { bool is_static_ = true; friend class ClassAccessor; + friend class DexFileVerifier; }; template <typename DataType> @@ -225,6 +227,10 @@ class ClassAccessor { return !(*this < rhs); } + const uint8_t* GetDataPointer() const { + return data_.ptr_pos_; + } + private: // Read data at current position. void ReadData() { @@ -244,14 +250,20 @@ class ClassAccessor { const uint32_t partition_pos_; // At iterator_end_, the iterator is no longer valid. const uint32_t iterator_end_; + + friend class DexFileVerifier; }; // Not explicit specifically for range-based loops. ALWAYS_INLINE ClassAccessor(const ClassIteratorData& data); - ClassAccessor(const DexFile& dex_file, const DexFile::ClassDef& class_def); + ALWAYS_INLINE ClassAccessor(const DexFile& dex_file, const DexFile::ClassDef& class_def); - ClassAccessor(const DexFile& dex_file, uint32_t class_def_index); + ALWAYS_INLINE ClassAccessor(const DexFile& dex_file, uint32_t class_def_index); + + ClassAccessor(const DexFile& dex_file, + const uint8_t* class_data, + uint32_t class_def_index = DexFile::kDexNoIndex32); // Return the code item for a method. const DexFile::CodeItem* GetCodeItem(const Method& method) const; @@ -354,6 +366,8 @@ class ClassAccessor { const uint32_t num_instance_fields_ = 0u; const uint32_t num_direct_methods_ = 0u; const uint32_t num_virtual_methods_ = 0u; + + friend class DexFileVerifier; }; } // namespace art diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h index 4e88ef6985..25cd2f4ddc 100644 --- a/libdexfile/dex/dex_file.h +++ b/libdexfile/dex/dex_file.h @@ -80,6 +80,7 @@ class DexFile { // The value of an invalid index. static const uint16_t kDexNoIndex16 = 0xFFFF; + static const uint32_t kDexNoIndex32 = 0xFFFFFFFF; // Raw header_item. struct Header { diff --git a/libdexfile/dex/dex_file_verifier.cc b/libdexfile/dex/dex_file_verifier.cc index fda6376454..fd011c86d1 100644 --- a/libdexfile/dex/dex_file_verifier.cc +++ b/libdexfile/dex/dex_file_verifier.cc @@ -23,6 +23,7 @@ #include "android-base/stringprintf.h" #include "base/leb128.h" +#include "class_accessor-inl.h" #include "code_item_accessors-inl.h" #include "descriptors_names.h" #include "dex_file-inl.h" @@ -614,9 +615,8 @@ bool DexFileVerifier::CheckClassDataItemMethod(uint32_t idx, uint32_t class_access_flags, dex::TypeIndex class_type_index, uint32_t code_offset, - ClassDataItemIterator* direct_it, - bool expect_direct) { - DCHECK_EQ(expect_direct, direct_it == nullptr); + ClassAccessor::Method* direct_method, + size_t* remaining_directs) { // Check for overflow. if (!CheckIndex(idx, header_->method_ids_size_, "class_data_item method_idx")) { return false; @@ -634,12 +634,14 @@ bool DexFileVerifier::CheckClassDataItemMethod(uint32_t idx, return false; } - // Check that it's not defined as both direct and virtual. - if (!expect_direct) { + // For virtual methods, we cross reference the method index to make sure it doesn't match any + // direct methods. + const bool expect_direct = direct_method == nullptr; + if (!expect_direct && *remaining_directs > 0) { // The direct methods are already known to be in ascending index order. So just keep up // with the current index. - for (; direct_it->HasNextDirectMethod(); direct_it->Next()) { - uint32_t direct_idx = direct_it->GetMemberIndex(); + while (true) { + const uint32_t direct_idx = direct_method->GetIndex(); if (direct_idx > idx) { break; } @@ -647,6 +649,11 @@ bool DexFileVerifier::CheckClassDataItemMethod(uint32_t idx, ErrorStringPrintf("Found virtual method with same index as direct method: %d", idx); return false; } + --*remaining_directs; + if (*remaining_directs == 0u) { + break; + } + direct_method->Read(); } } @@ -960,11 +967,14 @@ bool DexFileVerifier::CheckStaticFieldTypes(const DexFile::ClassDef* class_def) return true; } - ClassDataItemIterator field_it(*dex_file_, ptr_); + ClassAccessor accessor(*dex_file_, ptr_); EncodedStaticFieldValueIterator array_it(*dex_file_, *class_def); - for (; field_it.HasNextStaticField() && array_it.HasNext(); field_it.Next(), array_it.Next()) { - uint32_t index = field_it.GetMemberIndex(); + for (const ClassAccessor::Field& field : accessor.GetStaticFields()) { + if (!array_it.HasNext()) { + break; + } + uint32_t index = field.GetIndex(); const DexFile::TypeId& type_id = dex_file_->GetTypeId(dex_file_->GetFieldId(index).type_idx_); const char* field_type_name = dex_file_->GetStringData(dex_file_->GetStringId(type_id.descriptor_idx_)); @@ -1041,6 +1051,7 @@ bool DexFileVerifier::CheckStaticFieldTypes(const DexFile::ClassDef* class_def) ErrorStringPrintf("unexpected static field initial value type: %x", array_type); return false; } + array_it.Next(); } if (array_it.HasNext()) { @@ -1051,87 +1062,103 @@ bool DexFileVerifier::CheckStaticFieldTypes(const DexFile::ClassDef* class_def) } template <bool kStatic> -bool DexFileVerifier::CheckIntraClassDataItemFields(ClassDataItemIterator* it, +bool DexFileVerifier::CheckIntraClassDataItemFields(size_t count, + ClassAccessor::Field* field, bool* have_class, dex::TypeIndex* class_type_index, const DexFile::ClassDef** class_def) { - DCHECK(it != nullptr); + DCHECK(field != nullptr); constexpr const char* kTypeDescr = kStatic ? "static field" : "instance field"; - // These calls use the raw access flags to check whether the whole dex field is valid. + if (count == 0u) { + return true; + } + field->Read(); - if (!*have_class && (kStatic ? it->HasNextStaticField() : it->HasNextInstanceField())) { - *have_class = FindClassIndexAndDef(it->GetMemberIndex(), true, class_type_index, class_def); + if (!*have_class) { + *have_class = FindClassIndexAndDef(field->GetIndex(), true, class_type_index, class_def); if (!*have_class) { // Should have really found one. ErrorStringPrintf("could not find declaring class for %s index %" PRIu32, kTypeDescr, - it->GetMemberIndex()); + field->GetIndex()); return false; } } - DCHECK(*class_def != nullptr || - !(kStatic ? it->HasNextStaticField() : it->HasNextInstanceField())); + DCHECK(*class_def != nullptr); uint32_t prev_index = 0; - for (; kStatic ? it->HasNextStaticField() : it->HasNextInstanceField(); it->Next()) { - uint32_t curr_index = it->GetMemberIndex(); + for (size_t i = 0; ;) { + uint32_t curr_index = field->GetIndex(); + // These calls use the raw access flags to check whether the whole dex field is valid. if (!CheckOrder(kTypeDescr, curr_index, prev_index)) { return false; } if (!CheckClassDataItemField(curr_index, - it->GetRawMemberAccessFlags(), + field->GetRawAccessFlags(), (*class_def)->access_flags_, *class_type_index, kStatic)) { return false; } - + ++i; + if (i >= count) { + break; + } + field->Read(); prev_index = curr_index; } return true; } -template <bool kDirect> -bool DexFileVerifier::CheckIntraClassDataItemMethods( - ClassDataItemIterator* it, - ClassDataItemIterator* direct_it, - bool* have_class, - dex::TypeIndex* class_type_index, - const DexFile::ClassDef** class_def) { - DCHECK(it != nullptr); - constexpr const char* kTypeDescr = kDirect ? "direct method" : "virtual method"; +bool DexFileVerifier::CheckIntraClassDataItemMethods(ClassAccessor::Method* method, + size_t num_methods, + ClassAccessor::Method* direct_method, + size_t num_directs, + bool* have_class, + dex::TypeIndex* class_type_index, + const DexFile::ClassDef** class_def) { + DCHECK(method != nullptr); + const char* kTypeDescr = method->IsStaticOrDirect() ? "direct method" : "virtual method"; - if (!*have_class && (kDirect ? it->HasNextDirectMethod() : it->HasNextVirtualMethod())) { - *have_class = FindClassIndexAndDef(it->GetMemberIndex(), false, class_type_index, class_def); + if (num_methods == 0u) { + return true; + } + method->Read(); + + if (!*have_class) { + *have_class = FindClassIndexAndDef(method->GetIndex(), false, class_type_index, class_def); if (!*have_class) { // Should have really found one. ErrorStringPrintf("could not find declaring class for %s index %" PRIu32, kTypeDescr, - it->GetMemberIndex()); + method->GetIndex()); return false; } } - DCHECK(*class_def != nullptr || - !(kDirect ? it->HasNextDirectMethod() : it->HasNextVirtualMethod())); + DCHECK(*class_def != nullptr); uint32_t prev_index = 0; - for (; kDirect ? it->HasNextDirectMethod() : it->HasNextVirtualMethod(); it->Next()) { - uint32_t curr_index = it->GetMemberIndex(); + for (size_t i = 0; ;) { + uint32_t curr_index = method->GetIndex(); if (!CheckOrder(kTypeDescr, curr_index, prev_index)) { return false; } if (!CheckClassDataItemMethod(curr_index, - it->GetRawMemberAccessFlags(), + method->GetRawAccessFlags(), (*class_def)->access_flags_, *class_type_index, - it->GetMethodCodeItemOffset(), - direct_it, - kDirect)) { + method->GetCodeItemOffset(), + direct_method, + &num_directs)) { return false; } - + ++i; + if (i >= num_methods) { + break; + } + method->Read(); prev_index = curr_index; } @@ -1139,7 +1166,7 @@ bool DexFileVerifier::CheckIntraClassDataItemMethods( } bool DexFileVerifier::CheckIntraClassDataItem() { - ClassDataItemIterator it(*dex_file_, ptr_); + ClassAccessor accessor(*dex_file_, ptr_); // This code is complicated by the fact that we don't directly know which class this belongs to. // So we need to explicitly search with the first item we find (either field or method), and then, @@ -1148,14 +1175,18 @@ bool DexFileVerifier::CheckIntraClassDataItem() { dex::TypeIndex class_type_index; const DexFile::ClassDef* class_def = nullptr; + ClassAccessor::Field field(*dex_file_, accessor.ptr_pos_); // Check fields. - if (!CheckIntraClassDataItemFields<true>(&it, + if (!CheckIntraClassDataItemFields<true>(accessor.NumStaticFields(), + &field, &have_class, &class_type_index, &class_def)) { return false; } - if (!CheckIntraClassDataItemFields<false>(&it, + field.NextSection(); + if (!CheckIntraClassDataItemFields<false>(accessor.NumInstanceFields(), + &field, &have_class, &class_type_index, &class_def)) { @@ -1163,31 +1194,37 @@ bool DexFileVerifier::CheckIntraClassDataItem() { } // Check methods. - ClassDataItemIterator direct_it = it; - - if (!CheckIntraClassDataItemMethods<true>(&it, - nullptr /* direct_it */, - &have_class, - &class_type_index, - &class_def)) { + ClassAccessor::Method method(*dex_file_, field.ptr_pos_); + if (!CheckIntraClassDataItemMethods(&method, + accessor.NumDirectMethods(), + nullptr /* direct_it */, + 0u, + &have_class, + &class_type_index, + &class_def)) { return false; } - if (!CheckIntraClassDataItemMethods<false>(&it, - &direct_it, - &have_class, - &class_type_index, - &class_def)) { + ClassAccessor::Method direct_methods(*dex_file_, field.ptr_pos_); + method.NextSection(); + if (accessor.NumDirectMethods() != 0u) { + direct_methods.Read(); + } + if (!CheckIntraClassDataItemMethods(&method, + accessor.NumVirtualMethods(), + &direct_methods, + accessor.NumDirectMethods(), + &have_class, + &class_type_index, + &class_def)) { return false; } - const uint8_t* end_ptr = it.EndDataPointer(); - // Check static field types against initial static values in encoded array. if (!CheckStaticFieldTypes(class_def)) { return false; } - ptr_ = end_ptr; + ptr_ = method.ptr_pos_; return true; } @@ -1965,17 +2002,21 @@ bool DexFileVerifier::CheckOffsetToTypeMap(size_t offset, uint16_t type) { } dex::TypeIndex DexFileVerifier::FindFirstClassDataDefiner(const uint8_t* ptr, bool* success) { - ClassDataItemIterator it(*dex_file_, ptr); + ClassAccessor accessor(*dex_file_, ptr); *success = true; - if (it.HasNextStaticField() || it.HasNextInstanceField()) { - LOAD_FIELD(field, it.GetMemberIndex(), "first_class_data_definer field_id", + if (accessor.NumFields() != 0) { + ClassAccessor::Field read_field(*dex_file_, accessor.ptr_pos_); + read_field.Read(); + LOAD_FIELD(field, read_field.GetIndex(), "first_class_data_definer field_id", *success = false; return dex::TypeIndex(DexFile::kDexNoIndex16)) return field->class_idx_; } - if (it.HasNextMethod()) { - LOAD_METHOD(method, it.GetMemberIndex(), "first_class_data_definer method_id", + if (accessor.NumMethods() != 0) { + ClassAccessor::Method read_method(*dex_file_, accessor.ptr_pos_); + read_method.Read(); + LOAD_METHOD(method, read_method.GetIndex(), "first_class_data_definer method_id", *success = false; return dex::TypeIndex(DexFile::kDexNoIndex16)) return method->class_idx_; } @@ -2556,33 +2597,35 @@ bool DexFileVerifier::CheckInterAnnotationSetItem() { } bool DexFileVerifier::CheckInterClassDataItem() { - ClassDataItemIterator it(*dex_file_, ptr_); + ClassAccessor accessor(*dex_file_, ptr_); bool success; dex::TypeIndex defining_class = FindFirstClassDataDefiner(ptr_, &success); if (!success) { return false; } - for (; it.HasNextStaticField() || it.HasNextInstanceField(); it.Next()) { - LOAD_FIELD(field, it.GetMemberIndex(), "inter_class_data_item field_id", return false) + for (const ClassAccessor::Field& read_field : accessor.GetFields()) { + LOAD_FIELD(field, read_field.GetIndex(), "inter_class_data_item field_id", return false) if (UNLIKELY(field->class_idx_ != defining_class)) { ErrorStringPrintf("Mismatched defining class for class_data_item field"); return false; } } - for (; it.HasNextMethod(); it.Next()) { - uint32_t code_off = it.GetMethodCodeItemOffset(); + auto methods = accessor.GetMethods(); + auto it = methods.begin(); + for (; it != methods.end(); ++it) { + uint32_t code_off = it->GetCodeItemOffset(); if (code_off != 0 && !CheckOffsetToTypeMap(code_off, DexFile::kDexTypeCodeItem)) { return false; } - LOAD_METHOD(method, it.GetMemberIndex(), "inter_class_data_item method_id", return false) + LOAD_METHOD(method, it->GetIndex(), "inter_class_data_item method_id", return false) if (UNLIKELY(method->class_idx_ != defining_class)) { ErrorStringPrintf("Mismatched defining class for class_data_item method"); return false; } } - ptr_ = it.EndDataPointer(); + ptr_ = it.GetDataPointer(); return true; } diff --git a/libdexfile/dex/dex_file_verifier.h b/libdexfile/dex/dex_file_verifier.h index 43d1093809..79ddea43d4 100644 --- a/libdexfile/dex/dex_file_verifier.h +++ b/libdexfile/dex/dex_file_verifier.h @@ -22,6 +22,7 @@ #include "base/hash_map.h" #include "base/safe_map.h" +#include "class_accessor.h" #include "dex_file.h" #include "dex_file_types.h" @@ -90,8 +91,8 @@ class DexFileVerifier { uint32_t class_access_flags, dex::TypeIndex class_type_index, uint32_t code_offset, - ClassDataItemIterator* direct_it, - bool expect_direct); + ClassAccessor::Method* direct_method, + size_t* remaining_directs); ALWAYS_INLINE bool CheckOrder(const char* type_descr, uint32_t curr_index, uint32_t prev_index); bool CheckStaticFieldTypes(const DexFile::ClassDef* class_def); @@ -105,15 +106,17 @@ class DexFileVerifier { // Check all fields of the given type from the given iterator. Load the class data from the first // field, if necessary (and return it), or use the given values. template <bool kStatic> - bool CheckIntraClassDataItemFields(ClassDataItemIterator* it, + bool CheckIntraClassDataItemFields(size_t count, + ClassAccessor::Field* field, bool* have_class, dex::TypeIndex* class_type_index, const DexFile::ClassDef** class_def); // Check all methods of the given type from the given iterator. Load the class data from the first // method, if necessary (and return it), or use the given values. - template <bool kDirect> - bool CheckIntraClassDataItemMethods(ClassDataItemIterator* it, - ClassDataItemIterator* direct_it, + bool CheckIntraClassDataItemMethods(ClassAccessor::Method* method, + size_t num_methods, + ClassAccessor::Method* direct_method, + size_t num_directs, bool* have_class, dex::TypeIndex* class_type_index, const DexFile::ClassDef** class_def); |