From 78d639ef4be3ad7314846e1e6c1261d7d30f83fa Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 2 Sep 2014 11:17:34 -0700 Subject: [PATCH] ART: Tighten verifier list reading and offsets Check offsets and sizes for header entries of a dex file. Bug: 17347459 Change-Id: Ia1727c33dea51f7a8e345f3799f1ba414708239c --- runtime/dex_file_verifier.cc | 88 +++++++++++++++++++++++++++----------------- runtime/dex_file_verifier.h | 6 +++ 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc index d7d541a71..976cac942 100644 --- a/runtime/dex_file_verifier.cc +++ b/runtime/dex_file_verifier.cc @@ -171,7 +171,7 @@ bool DexFileVerifier::CheckShortyDescriptorMatch(char shorty_char, const char* d } bool DexFileVerifier::CheckListSize(const void* start, size_t count, size_t elem_size, - const char* label) { + const char* label) { // Check that size is not 0. CHECK_NE(elem_size, 0U); @@ -201,6 +201,23 @@ bool DexFileVerifier::CheckListSize(const void* start, size_t count, size_t elem return true; } +bool DexFileVerifier::CheckList(size_t element_size, const char* label, const byte* *ptr) { + // Check that the list is available. The first 4B are the count. + if (!CheckListSize(*ptr, 1, 4U, label)) { + return false; + } + + uint32_t count = *reinterpret_cast(*ptr); + if (count > 0) { + if (!CheckListSize(*ptr + 4, count, element_size, label)) { + return false; + } + } + + *ptr += 4 + count * element_size; + return true; +} + bool DexFileVerifier::CheckIndex(uint32_t field, uint32_t limit, const char* label) { if (UNLIKELY(field >= limit)) { ErrorStringPrintf("Bad index for %s: %x >= %x", label, field, limit); @@ -209,6 +226,20 @@ bool DexFileVerifier::CheckIndex(uint32_t field, uint32_t limit, const char* lab return true; } +bool DexFileVerifier::CheckValidOffsetAndSize(uint32_t offset, uint32_t size, const char* label) { + if (size == 0) { + if (offset != 0) { + ErrorStringPrintf("Offset(%d) should be zero when size is zero for %s.", offset, label); + return false; + } + } + if (size_ <= offset) { + ErrorStringPrintf("Offset(%d) should be within file size(%zu) for %s.", offset, size_, label); + return false; + } + return true; +} + bool DexFileVerifier::CheckHeader() { // Check file size from the header. uint32_t expected_size = header_->file_size_; @@ -238,11 +269,29 @@ bool DexFileVerifier::CheckHeader() { return false; } - return true; + // Check that all offsets are inside the file. + bool result = + CheckValidOffsetAndSize(header_->link_off_, header_->link_size_, "link") && + CheckValidOffsetAndSize(header_->map_off_, header_->map_off_, "map") && + CheckValidOffsetAndSize(header_->string_ids_off_, header_->string_ids_size_, "string-ids") && + CheckValidOffsetAndSize(header_->type_ids_off_, header_->type_ids_size_, "type-ids") && + CheckValidOffsetAndSize(header_->proto_ids_off_, header_->proto_ids_size_, "proto-ids") && + CheckValidOffsetAndSize(header_->field_ids_off_, header_->field_ids_size_, "field-ids") && + CheckValidOffsetAndSize(header_->method_ids_off_, header_->method_ids_size_, "method-ids") && + CheckValidOffsetAndSize(header_->class_defs_off_, header_->class_defs_size_, "class-defs") && + CheckValidOffsetAndSize(header_->data_off_, header_->data_size_, "data"); + + return result; } bool DexFileVerifier::CheckMap() { - const DexFile::MapList* map = reinterpret_cast(begin_ + header_->map_off_); + const DexFile::MapList* map = reinterpret_cast(begin_ + + header_->map_off_); + // Check that map list content is available. + if (!CheckListSize(map, 1, sizeof(DexFile::MapList), "maplist content")) { + return false; + } + const DexFile::MapItem* item = map->list_; uint32_t count = map->size_; @@ -1117,48 +1166,21 @@ bool DexFileVerifier::CheckIntraSectionIterate(size_t offset, uint32_t section_c break; } case DexFile::kDexTypeTypeList: { - const DexFile::TypeList* list = reinterpret_cast(ptr_); - - // Check that at least the header (size) is available. - if (!CheckListSize(list, 1, DexFile::TypeList::GetHeaderSize(), "type_list")) { + if (!CheckList(sizeof(DexFile::TypeItem), "type_list", &ptr_)) { return false; } - - // Check that the whole list is available. - const size_t list_size = DexFile::TypeList::GetListSize(list->Size()); - if (!CheckListSize(list, 1, list_size, "type_list size")) { - return false; - } - - ptr_ += list_size; break; } case DexFile::kDexTypeAnnotationSetRefList: { - const DexFile::AnnotationSetRefList* list = - reinterpret_cast(ptr_); - const DexFile::AnnotationSetRefItem* item = list->list_; - uint32_t count = list->size_; - - if (!CheckListSize(list, 1, sizeof(DexFile::AnnotationSetRefList), - "annotation_set_ref_list") || - !CheckListSize(item, count, sizeof(DexFile::AnnotationSetRefItem), - "annotation_set_ref_list size")) { + if (!CheckList(sizeof(DexFile::AnnotationSetRefItem), "annotation_set_ref_list", &ptr_)) { return false; } - ptr_ = reinterpret_cast(item + count); break; } case DexFile::kDexTypeAnnotationSetItem: { - const DexFile::AnnotationSetItem* set = - reinterpret_cast(ptr_); - const uint32_t* item = set->entries_; - uint32_t count = set->size_; - - if (!CheckListSize(set, 1, sizeof(DexFile::AnnotationSetItem), "annotation_set_item") || - !CheckListSize(item, count, sizeof(uint32_t), "annotation_set_item size")) { + if (!CheckList(sizeof(uint32_t), "annotation_set_item", &ptr_)) { return false; } - ptr_ = reinterpret_cast(item + count); break; } case DexFile::kDexTypeClassDataItem: { diff --git a/runtime/dex_file_verifier.h b/runtime/dex_file_verifier.h index 1d915b93f..606da5422 100644 --- a/runtime/dex_file_verifier.h +++ b/runtime/dex_file_verifier.h @@ -43,6 +43,12 @@ class DexFileVerifier { bool CheckShortyDescriptorMatch(char shorty_char, const char* descriptor, bool is_return_type); bool CheckListSize(const void* start, size_t count, size_t element_size, const char* label); + // Check a list. The head is assumed to be at *ptr, and elements to be of size element_size. If + // successful, the ptr will be moved forward the amount covered by the list. + bool CheckList(size_t element_size, const char* label, const byte* *ptr); + // Checks whether the offset is zero (when size is zero) or that the offset falls within the area + // claimed by the file. + bool CheckValidOffsetAndSize(uint32_t offset, uint32_t size, const char* label); bool CheckIndex(uint32_t field, uint32_t limit, const char* label); bool CheckHeader(); -- 2.11.0