diff options
author | Nicolas Geoffray <ngeoffray@google.com> | 2021-09-09 17:06:46 +0100 |
---|---|---|
committer | Nicolas Geoffray <ngeoffray@google.com> | 2021-09-13 12:56:37 +0000 |
commit | 4667b35dbf14877ced91b77375e1acd922b02bbf (patch) | |
tree | 3af732d091176ddcd49208b3dc2d2d67866dea8f | |
parent | b2e6ae0e8b86c2bde220eb31e841ab8cd9bb4127 (diff) |
vdex: add checks in the event of file corruption.
It's unclear yet why the vdex files are being corrupted. But system
server, which is reading these vdex files should be robust to any vdex
corruption.
Bug: 199309980
Bug: 199395272
Test: test.py
(cherry picked from commit a74a7071490e47e1b5590cc19726f1620fd0ee43)
(cherry picked from commit a35586522fd398fb2845a9ae8427aa4853f155be)
Merged-In: Ia85ab8b23a0be4069cfa058a86fdf561f1ceb432
Change-Id: I558238fd7cc0d7bc2f89f989ad53db8eb7a2eb24
-rw-r--r-- | libdexfile/dex/dex_file.h | 2 | ||||
-rw-r--r-- | runtime/oat_file.cc | 110 | ||||
-rw-r--r-- | runtime/vdex_file.cc | 3 | ||||
-rw-r--r-- | runtime/vdex_file.h | 5 |
4 files changed, 104 insertions, 16 deletions
diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h index 5363b00c24..e3027fc95f 100644 --- a/libdexfile/dex/dex_file.h +++ b/libdexfile/dex/dex_file.h @@ -738,7 +738,7 @@ class DexFile { } // Used by oat writer. - void SetOatDexFile(OatDexFile* oat_dex_file) const { + void SetOatDexFile(const OatDexFile* oat_dex_file) const { oat_dex_file_ = oat_dex_file; } diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 8f653c2282..14e7a1b60e 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -168,7 +168,7 @@ class OatFileBase : public OatFile { bool Setup(int zip_fd, ArrayRef<const std::string> dex_filenames, std::string* error_msg); - void Setup(const std::vector<const DexFile*>& dex_files); + bool Setup(const std::vector<const DexFile*>& dex_files, std::string* error_msg); // Setters exposed for ElfOatFile. @@ -476,18 +476,71 @@ static bool ReadIndexBssMapping(OatFile* oat_file, return true; } -void OatFileBase::Setup(const std::vector<const DexFile*>& dex_files) { +static bool ComputeAndCheckTypeLookupTableData(const DexFile::Header& header, + const uint8_t* type_lookup_table_start, + const VdexFile* vdex_file, + const uint8_t** type_lookup_table_data, + std::string* error_msg) { + if (type_lookup_table_start == nullptr || + reinterpret_cast<const uint32_t*>(type_lookup_table_start)[0] == 0) { + *type_lookup_table_data = nullptr; + return true; + } + + *type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + size_t expected_table_size = TypeLookupTable::RawDataLength(header.class_defs_size_); + size_t found_size = reinterpret_cast<const uint32_t*>(type_lookup_table_start)[0]; + if (UNLIKELY(found_size != expected_table_size)) { + *error_msg = + StringPrintf("In vdex file '%s' unexpected type lookup table size: found %zu, expected %zu", + vdex_file->GetName().c_str(), + found_size, + expected_table_size); + return false; + } + if (UNLIKELY(!vdex_file->Contains(*type_lookup_table_data))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid type lookup table pointer %p not in [%p, %p]", + vdex_file->GetName().c_str(), + type_lookup_table_data, + vdex_file->Begin(), + vdex_file->End()); + return false; + } + if (UNLIKELY(!vdex_file->Contains(*type_lookup_table_data + expected_table_size - 1))) { + *error_msg = + StringPrintf("In vdex file '%s' found overflowing type lookup table %p not in [%p, %p]", + vdex_file->GetName().c_str(), + type_lookup_table_data + expected_table_size, + vdex_file->Begin(), + vdex_file->End()); + return false; + } + if (UNLIKELY(!IsAligned<4>(type_lookup_table_start))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid type lookup table alignment %p", + vdex_file->GetName().c_str(), + type_lookup_table_start); + return false; + } + return true; +} + +bool OatFileBase::Setup(const std::vector<const DexFile*>& dex_files, std::string* error_msg) { uint32_t i = 0; const uint8_t* type_lookup_table_start = nullptr; for (const DexFile* dex_file : dex_files) { - type_lookup_table_start = vdex_->GetNextTypeLookupTableData(type_lookup_table_start, i++); std::string dex_location = dex_file->GetLocation(); std::string canonical_location = DexFileLoader::GetDexCanonicalLocation(dex_location.c_str()); + type_lookup_table_start = vdex_->GetNextTypeLookupTableData(type_lookup_table_start, i++); const uint8_t* type_lookup_table_data = nullptr; - if (type_lookup_table_start != nullptr && - (reinterpret_cast<uint32_t*>(type_lookup_table_start[0]) != 0)) { - type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + if (!ComputeAndCheckTypeLookupTableData(dex_file->GetHeader(), + type_lookup_table_start, + vdex_.get(), + &type_lookup_table_data, + error_msg)) { + return false; } // Create an OatDexFile and add it to the owning container. OatDexFile* oat_dex_file = new OatDexFile( @@ -497,7 +550,6 @@ void OatFileBase::Setup(const std::vector<const DexFile*>& dex_files) { dex_location, canonical_location, type_lookup_table_data); - dex_file->SetOatDexFile(oat_dex_file); oat_dex_files_storage_.push_back(oat_dex_file); // Add the location and canonical location (if different) to the oat_dex_files_ table. @@ -508,6 +560,11 @@ void OatFileBase::Setup(const std::vector<const DexFile*>& dex_files) { oat_dex_files_.Put(canonical_key, oat_dex_file); } } + // Now that we've created all the OatDexFile, update the dex files. + for (i = 0; i < dex_files.size(); ++i) { + dex_files[i]->SetOatDexFile(oat_dex_files_storage_[i]); + } + return true; } bool OatFileBase::Setup(int zip_fd, @@ -1583,7 +1640,11 @@ class OatFileBackedByVdex final : public OatFileBase { oat_file->SetVdex(vdex_file.release()); oat_file->SetupHeader(dex_files.size()); // Initialize OatDexFiles. - oat_file->Setup(dex_files); + std::string error_msg; + if (!oat_file->Setup(dex_files, &error_msg)) { + LOG(WARNING) << "Could not create in-memory vdex file: " << error_msg; + return nullptr; + } return oat_file.release(); } @@ -1601,6 +1662,25 @@ class OatFileBackedByVdex final : public OatFileBase { for (const uint8_t* dex_file_start = vdex_file->GetNextDexFileData(nullptr, i); dex_file_start != nullptr; dex_file_start = vdex_file->GetNextDexFileData(dex_file_start, ++i)) { + const DexFile::Header* header = reinterpret_cast<const DexFile::Header*>(dex_file_start); + if (UNLIKELY(!vdex_file->Contains(dex_file_start))) { + *error_msg = + StringPrintf("In vdex file '%s' found invalid dex file pointer %p not in [%p, %p]", + dex_location.c_str(), + dex_file_start, + vdex_file->Begin(), + vdex_file->End()); + return nullptr; + } + if (UNLIKELY(!vdex_file->Contains(dex_file_start + header->file_size_ - 1))) { + *error_msg = + StringPrintf("In vdex file '%s' found overflowing dex file %p not in [%p, %p]", + dex_location.c_str(), + dex_file_start + header->file_size_, + vdex_file->Begin(), + vdex_file->End()); + return nullptr; + } if (UNLIKELY(!DexFileLoader::IsVersionAndMagicValid(dex_file_start))) { *error_msg = StringPrintf("In vdex file '%s' found dex file with invalid dex file version", @@ -1612,10 +1692,14 @@ class OatFileBackedByVdex final : public OatFileBase { std::string canonical_location = DexFileLoader::GetDexCanonicalLocation(location.c_str()); type_lookup_table_start = vdex_file->GetNextTypeLookupTableData(type_lookup_table_start, i); const uint8_t* type_lookup_table_data = nullptr; - if (type_lookup_table_start != nullptr && - (reinterpret_cast<uint32_t*>(type_lookup_table_start[0]) != 0)) { - type_lookup_table_data = type_lookup_table_start + sizeof(uint32_t); + if (!ComputeAndCheckTypeLookupTableData(*header, + type_lookup_table_start, + vdex_file, + &type_lookup_table_data, + error_msg)) { + return nullptr; } + OatDexFile* oat_dex_file = new OatDexFile(oat_file.get(), dex_file_start, vdex_file->GetLocationChecksum(i), @@ -1656,7 +1740,9 @@ class OatFileBackedByVdex final : public OatFileBase { return nullptr; } oat_file->SetupHeader(oat_file->external_dex_files_.size()); - oat_file->Setup(MakeNonOwningPointerVector(oat_file->external_dex_files_)); + if (!oat_file->Setup(MakeNonOwningPointerVector(oat_file->external_dex_files_), error_msg)) { + return nullptr; + } } return oat_file.release(); diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc index 29efd4016f..967167961c 100644 --- a/runtime/vdex_file.cc +++ b/runtime/vdex_file.cc @@ -192,7 +192,8 @@ const uint8_t* VdexFile::GetNextTypeLookupTableData(const uint8_t* cursor, } else { const uint8_t* data = cursor + sizeof(uint32_t) + reinterpret_cast<const uint32_t*>(cursor)[0]; // TypeLookupTables are required to be 4 byte aligned. the OatWriter makes sure they are. - CHECK_ALIGNED(data, 4); + // We don't check this here to be defensive against corrupted vdex files. + // Callers should check the returned value matches their expectations. return data; } } diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h index eb8b81742b..a66ff88fb2 100644 --- a/runtime/vdex_file.h +++ b/runtime/vdex_file.h @@ -157,8 +157,6 @@ class VdexFile { return size; } - bool IsDexSectionValid() const; - bool HasDexSection() const { return GetSectionHeader(VdexSection::kDexFileSection).section_size != 0u; } @@ -251,6 +249,9 @@ class VdexFile { const uint8_t* Begin() const { return mmap_.Begin(); } const uint8_t* End() const { return mmap_.End(); } size_t Size() const { return mmap_.Size(); } + bool Contains(const uint8_t* pointer) const { + return pointer >= Begin() && pointer < End(); + } const VdexFileHeader& GetVdexFileHeader() const { return *reinterpret_cast<const VdexFileHeader*>(Begin()); |