From de776d4ef06ca29c240de3444348894f032b03ff Mon Sep 17 00:00:00 2001
From: Lei Zhang <leizleiz@users.noreply.github.com>
Date: Wed, 23 Sep 2015 16:06:28 -0700
Subject: Check for integer overflow in sfntly::FontData::Bound().

Also delete dead code and cleanup some nits.

This is cl/96914065.
---
 cpp/src/sfntly/data/font_data.cc | 44 ++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

(limited to 'cpp/src/sfntly/data/font_data.cc')

diff --git a/cpp/src/sfntly/data/font_data.cc b/cpp/src/sfntly/data/font_data.cc
index d2b95ea..95bee3e 100644
--- a/cpp/src/sfntly/data/font_data.cc
+++ b/cpp/src/sfntly/data/font_data.cc
@@ -14,11 +14,13 @@
  * limitations under the License.
  */
 
-#include <limits.h>
+#include "sfntly/data/font_data.h"
+
 #include <algorithm>
 #include <functional>
+#include <limits>
 
-#include "sfntly/data/font_data.h"
+#include "sfntly/port/logging.h"
 
 namespace sfntly {
 
@@ -26,21 +28,29 @@ int32_t FontData::Size() const {
   return std::min<int32_t>(array_->Size() - bound_offset_, bound_length_);
 }
 
-bool FontData::Bound(int32_t offset, int32_t length) {
-  if (offset + length > Size() || offset < 0 || length < 0)
-    return false;
-
-  bound_offset_ += offset;
+void FontData::Bound(int32_t offset, int32_t length) {
+  // Inputs should not be negative.
+  CHECK(offset >= 0);
+  CHECK(length >= 0);
+
+  // Check to make sure |bound_offset_| will not overflow.
+  CHECK(bound_offset_ <= std::numeric_limits<int32_t>::max() - offset);
+  const int32_t new_offset = bound_offset_ + offset;
+
+  if (length == GROWABLE_SIZE) {
+    // When |length| has the special value of GROWABLE_SIZE, it means the size
+    // should not have any artificial limits, thus it is just the underlying
+    // |array_|'s size. Just make sure |new_offset| is still within bounds.
+    CHECK(new_offset <= array_->Size());
+  } else {
+    // When |length| has any other value, |new_offset| + |length| points to the
+    // end of the array. Make sure that is within bounds, but use subtraction to
+    // avoid an integer overflow.
+    CHECK(new_offset <= array_->Size() - length);
+  }
+
+  bound_offset_ = new_offset;
   bound_length_ = length;
-  return true;
-}
-
-bool FontData::Bound(int32_t offset) {
-if (offset > Size() || offset < 0)
-    return false;
-
-  bound_offset_ += offset;
-  return true;
 }
 
 int32_t FontData::Length() const {
@@ -60,7 +70,7 @@ FontData::FontData(FontData* data, int32_t offset) {
   Init(data->array_);
   Bound(data->bound_offset_ + offset,
         (data->bound_length_ == GROWABLE_SIZE)
-            ? GROWABLE_SIZE : data->bound_length_ - offset);
+        ? GROWABLE_SIZE : data->bound_length_ - offset);
 }
 
 FontData::~FontData() {}
-- 
cgit v1.2.3