Add range and arithmetic checks to `base::VectorBuffer`.

Bug: 817982
Change-Id: If8602db9f284873f037775f3a37bcac8e120b313
Reviewed-on: https://chromium-review.googlesource.com/1166456Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583155}

base/containers/vector_buffer.h

@@ -13,6 +13,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/numerics/checked_math.h"
 
 namespace base {
 namespace internal {
@@ -45,7 +46,8 @@ class VectorBuffer {
   __attribute__((no_sanitize("cfi-unrelated-cast", "vptr")))
 #endif
   VectorBuffer(size_t count)
-      : buffer_(reinterpret_cast<T*>(malloc(sizeof(T) * count))),
+      : buffer_(reinterpret_cast<T*>(
+            malloc(CheckMul(sizeof(T), count).ValueOrDie()))),
         capacity_(count) {
   }
   VectorBuffer(VectorBuffer&& other) noexcept
@@ -68,8 +70,21 @@ class VectorBuffer {
 
   size_t capacity() const { return capacity_; }
 
-  T& operator[](size_t i) { return buffer_[i]; }
-  const T& operator[](size_t i) const { return buffer_[i]; }
+  T& operator[](size_t i) {
+    // TODO(crbug.com/817982): Some call sites (at least circular_deque.h) are
+    // calling this with `i == capacity_` as a way of getting `end()`. Therefore
+    // we have to allow this for now (`i <= capacity_`), until we fix those call
+    // sites to use real iterators. This comment applies here and to `const T&
+    // operator[]`, below.
+    CHECK_LE(i, capacity_);
+    return buffer_[i];
+  }
+
+  const T& operator[](size_t i) const {
+    CHECK_LE(i, capacity_);
+    return buffer_[i];
+  }
+
   T* begin() { return buffer_; }
   T* end() { return &buffer_[capacity_]; }
 
@@ -87,6 +102,7 @@ class VectorBuffer {
             typename std::enable_if<!std::is_trivially_destructible<T2>::value,
                                     int>::type = 0>
   void DestructRange(T* begin, T* end) {
+    CHECK_LE(begin, end);
     while (begin != end) {
       begin->~T();
       begin++;
@@ -108,8 +124,10 @@ class VectorBuffer {
             typename std::enable_if<base::is_trivially_copyable<T2>::value,
                                     int>::type = 0>
   static void MoveRange(T* from_begin, T* from_end, T* to) {
-    DCHECK(!RangesOverlap(from_begin, from_end, to));
-    memcpy(to, from_begin, (from_end - from_begin) * sizeof(T));
+    CHECK(!RangesOverlap(from_begin, from_end, to));
+    memcpy(
+        to, from_begin,
+        CheckSub(get_uintptr(from_end), get_uintptr(from_begin)).ValueOrDie());
   }
 
   // Not trivially copyable, but movable: call the move constructor and
@@ -119,7 +137,7 @@ class VectorBuffer {
                                         !base::is_trivially_copyable<T2>::value,
                                     int>::type = 0>
   static void MoveRange(T* from_begin, T* from_end, T* to) {
-    DCHECK(!RangesOverlap(from_begin, from_end, to));
+    CHECK(!RangesOverlap(from_begin, from_end, to));
     while (from_begin != from_end) {
       new (to) T(std::move(*from_begin));
       from_begin->~T();
@@ -135,7 +153,7 @@ class VectorBuffer {
                                         !base::is_trivially_copyable<T2>::value,
                                     int>::type = 0>
   static void MoveRange(T* from_begin, T* from_end, T* to) {
-    DCHECK(!RangesOverlap(from_begin, from_end, to));
+    CHECK(!RangesOverlap(from_begin, from_end, to));
     while (from_begin != from_end) {
       new (to) T(*from_begin);
       from_begin->~T();
@@ -145,10 +163,22 @@ class VectorBuffer {
   }
 
  private:
+  // TODO(crbug.com/817982): What we really need is for checked_math.h to be
+  // able to do checked arithmetic on pointers.
+  static inline uintptr_t get_uintptr(const T* t) {
+    return reinterpret_cast<uintptr_t>(t);
+  }
+
   static bool RangesOverlap(const T* from_begin,
                             const T* from_end,
                             const T* to) {
-    return !(to >= from_end || to + (from_end - from_begin) <= from_begin);
+    const auto from_begin_uintptr = get_uintptr(from_begin);
+    const auto from_end_uintptr = get_uintptr(from_end);
+    const auto to_uintptr = get_uintptr(to);
+    return !(
+        to >= from_end ||
+        CheckAdd(to_uintptr, CheckSub(from_end_uintptr, from_begin_uintptr))
+                .ValueOrDie() <= from_begin_uintptr);
   }
 
   T* buffer_ = nullptr;