From 7b105f83ab3e644a6ffcb971df65803c18c61bff Mon Sep 17 00:00:00 2001 From: Justin Cohen Date: Thu, 28 Jul 2022 21:18:25 -0400 Subject: [PATCH] ios: Properly handle overflows in scoped_vm_read. Passing -1 (or size_t max) to ScopedVMRead would succeed, because the amount of memory to be read would overflow vm_address_t/vm_size_t and turn into something reasonable. ScopedVMRead would return true having only read a miniscule subset of the requested data length. Bug: 1348341 Change-Id: I061a1d86928f211c541a6378a78ee045d489a838 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3791710 Commit-Queue: Justin Cohen Reviewed-by: Mark Mentovai --- util/ios/scoped_vm_read.cc | 4 ++++ util/ios/scoped_vm_read_test.cc | 1 + 2 files changed, 5 insertions(+) diff --git a/util/ios/scoped_vm_read.cc b/util/ios/scoped_vm_read.cc index 06cf4344..7ae85df7 100644 --- a/util/ios/scoped_vm_read.cc +++ b/util/ios/scoped_vm_read.cc @@ -43,6 +43,10 @@ bool ScopedVMReadInternal::Read(const void* data, const size_t data_length) { vm_address_t page_region_address = trunc_page(data_address); vm_size_t page_region_size = round_page(data_address - page_region_address + data_length); + if (page_region_size < data_length) { + CRASHPAD_RAW_LOG("ScopedVMRead data_length overflow"); + return false; + } kern_return_t kr = vm_read(mach_task_self(), page_region_address, page_region_size, diff --git a/util/ios/scoped_vm_read_test.cc b/util/ios/scoped_vm_read_test.cc index 80edf042..072d4010 100644 --- a/util/ios/scoped_vm_read_test.cc +++ b/util/ios/scoped_vm_read_test.cc @@ -30,6 +30,7 @@ TEST(ScopedVMReadTest, BasicFunctionality) { ASSERT_FALSE(vmread_bad.Read(reinterpret_cast(0x1000), 100)); vm_address_t address = 1; ASSERT_FALSE(vmread_bad.Read(&address, 1000000000)); + ASSERT_FALSE(vmread_bad.Read(&address, -1)); // array constexpr char read_me[] = "read me";