From 60ff012872556b2a8ef68422071976c24a5075c3 Mon Sep 17 00:00:00 2001
From: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri, 21 Dec 2018 13:06:12 -0800
Subject: [PATCH] Change ProcessMemory to accept VMSizes

As Mark noted in [1] ProcessMemory should accept VMSize instead of
size_t, the two types can differ on platforms where a cross-bitness
handler could cause a 32-bit handler to inspect a 64-bit process. By
centralizing the checks in ProcessMemory, we can leave the individual
platform-specific implementations (in ProcessMemory*::ReadUpTo) to
accept size_ts.

[1] crrev.com/c/1388017/2/snapshot/crashpad_types/crashpad_info_reader.cc#70

Bug: crashpad:270
Change-Id: I3aab483221de36f3b1478cb9503101b142dae681
Reviewed-on: https://chromium-review.googlesource.com/c/1387756
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
---
 util/process/process_memory.cc | 31 ++++++++++++++++++++++---------
 util/process/process_memory.h  |  6 +++---
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/util/process/process_memory.cc b/util/process/process_memory.cc
index c23af93e..ab87b940 100644
--- a/util/process/process_memory.cc
+++ b/util/process/process_memory.cc
@@ -17,13 +17,20 @@
 #include <algorithm>
 
 #include "base/logging.h"
+#include "util/numeric/safe_assignment.h"
 
 namespace crashpad {
 
-bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
+bool ProcessMemory::Read(VMAddress address, VMSize size, void* buffer) const {
+  size_t local_size;
+  if (!AssignIfInRange(&local_size, size)) {
+    LOG(ERROR) << "size " << size << " out of bounds for size_t";
+    return false;
+  }
+
   char* buffer_c = static_cast<char*>(buffer);
-  while (size > 0) {
-    ssize_t bytes_read = ReadUpTo(address, size, buffer_c);
+  while (local_size > 0) {
+    ssize_t bytes_read = ReadUpTo(address, local_size, buffer_c);
     if (bytes_read < 0) {
       return false;
     }
@@ -31,8 +38,8 @@ bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
       LOG(ERROR) << "short read";
       return false;
     }
-    DCHECK_LE(static_cast<size_t>(bytes_read), size);
-    size -= bytes_read;
+    DCHECK_LE(static_cast<size_t>(bytes_read), local_size);
+    local_size -= bytes_read;
     address += bytes_read;
     buffer_c += bytes_read;
   }
@@ -41,15 +48,21 @@ bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
 
 bool ProcessMemory::ReadCStringInternal(VMAddress address,
                                         bool has_size,
-                                        size_t size,
+                                        VMSize size,
                                         std::string* string) const {
+  size_t local_size;
+  if (!AssignIfInRange(&local_size, size)) {
+    LOG(ERROR) << "size " << size << " out of bounds for size_t";
+    return false;
+  }
+
   string->clear();
 
   char buffer[4096];
   do {
     size_t read_size;
     if (has_size) {
-      read_size = std::min(sizeof(buffer), size);
+      read_size = std::min(sizeof(buffer), local_size);
     } else {
       read_size = sizeof(buffer);
     }
@@ -70,8 +83,8 @@ bool ProcessMemory::ReadCStringInternal(VMAddress address,
     string->append(buffer, bytes_read);
 
     address += bytes_read;
-    size -= bytes_read;
-  } while (!has_size || size > 0);
+    local_size -= bytes_read;
+  } while (!has_size || local_size > 0);
 
   LOG(ERROR) << "unterminated string";
   return false;
diff --git a/util/process/process_memory.h b/util/process/process_memory.h
index 5ea595ef..32e7472f 100644
--- a/util/process/process_memory.h
+++ b/util/process/process_memory.h
@@ -46,7 +46,7 @@ class ProcessMemory {
   //!
   //! \return `true` on success, with \a buffer filled appropriately. `false` on
   //!     failure, with a message logged.
-  bool Read(VMAddress address, size_t size, void* buffer) const;
+  bool Read(VMAddress address, VMSize size, void* buffer) const;
 
   //! \brief Reads a `NUL`-terminated C string from the target process into a
   //!     string in the current process.
@@ -79,7 +79,7 @@ class ProcessMemory {
   //!     a `NUL` terminator is not found within \a size bytes, or when
   //!     encountering unmapped or unreadable pages.
   bool ReadCStringSizeLimited(VMAddress address,
-                              size_t size,
+                              VMSize size,
                               std::string* string) const {
     return ReadCStringInternal(address, true, size, string);
   }
@@ -124,7 +124,7 @@ class ProcessMemory {
   //!     encountering unmapped or unreadable pages.
   virtual bool ReadCStringInternal(VMAddress address,
                                    bool has_size,
-                                   size_t size,
+                                   VMSize size,
                                    std::string* string) const;
 };