Sanitize values in chrome_pdf::PDFiumPage::PageToScreen().

pdf/pdfium/pdfium_page.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "pdf/pdfium/pdfium_page.h"
 
 #include <math.h>
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 
 #include "base/logging.h"
+#include "base/numerics/safe_math.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "pdf/pdfium/pdfium_api_string_buffer_adapter.h"
 #include "pdf/pdfium/pdfium_engine.h"
 #include "printing/units.h"
 
 // Used when doing hit detection.
 #define kTolerance 20.0
 
@@ skipping 429 matching lines @@
 pp::Rect PDFiumPage::PageToScreen(const pp::Point& offset,
                                   double zoom,
                                   double left,
                                   double top,
                                   double right,
                                   double bottom,
                                   int rotation) const {
   if (!available_)
     return pp::Rect();
 
-  int new_left, new_top, new_right, new_bottom;
-  FPDF_PageToDevice(
-      page_,
-      static_cast<int>((rect_.x() - offset.x()) * zoom),
-      static_cast<int>((rect_.y() - offset.y()) * zoom),
-      static_cast<int>(ceil(rect_.width() * zoom)),
-      static_cast<int>(ceil(rect_.height() * zoom)),
-      rotation, left, top, &new_left, &new_top);
-  FPDF_PageToDevice(
-      page_,
-      static_cast<int>((rect_.x() - offset.x()) * zoom),
-      static_cast<int>((rect_.y() - offset.y()) * zoom),
-      static_cast<int>(ceil(rect_.width() * zoom)),
-      static_cast<int>(ceil(rect_.height() * zoom)),
-      rotation, right, bottom, &new_right, &new_bottom);
+  base::CheckedNumeric<double> start_x = rect_.x();

[Tom Sepez, 2016/09/27 15:57:17]

can we get away with just using doubles here?  I'm not sure how they would
overflow.

[Lei Zhang, 2016/09/27 16:44:40]

Can multiplying by |zoom| overflow? Am I being too paranoid?

[Tom Sepez, 2016/09/27 16:46:42]

Probably can, but I wouldn't worry about it.  There are too many places where
doubles would overflow throughout the code ...

[Lei Zhang, 2016/09/27 17:50:44]

Done.

+  start_x -= offset.x();
+  start_x *= zoom;
+  base::CheckedNumeric<double> start_y = rect_.y();
+  start_y -= offset.y();
+  start_y *= zoom;
+  base::CheckedNumeric<double> size_x = rect_.width();
+  size_x *= zoom;
+  base::CheckedNumeric<double> size_y = rect_.height();
+  size_y *= zoom;
+  if (!start_x.IsValid() || !start_y.IsValid() || !size_x.IsValid() ||
+      !size_y.IsValid()) {
+    return pp::Rect();
+  }
+  if (!base::IsValueInRangeForNumericType<int>(start_x.ValueOrDie()) ||
+      !base::IsValueInRangeForNumericType<int>(start_y.ValueOrDie()) ||
+      !base::IsValueInRangeForNumericType<int>(size_x.ValueOrDie()) ||
+      !base::IsValueInRangeForNumericType<int>(size_y.ValueOrDie())) {
+    return pp::Rect();
+  }
+
+  int new_left;
+  int new_top;
+  int new_right;
+  int new_bottom;
+  FPDF_PageToDevice(page_, static_cast<int>(start_x.ValueOrDie()),
+                    static_cast<int>(start_y.ValueOrDie()),
+                    static_cast<int>(ceil(size_x.ValueOrDie())),
+                    static_cast<int>(ceil(size_y.ValueOrDie())), rotation, left,
+                    top, &new_left, &new_top);
+  FPDF_PageToDevice(page_, static_cast<int>(start_x.ValueOrDie()),
+                    static_cast<int>(start_y.ValueOrDie()),
+                    static_cast<int>(ceil(size_x.ValueOrDie())),
+                    static_cast<int>(ceil(size_y.ValueOrDie())), rotation,
+                    right, bottom, &new_right, &new_bottom);
 
   // If the PDF is rotated, the horizontal/vertical coordinates could be
   // flipped.  See
   // http://www.netl.doe.gov/publications/proceedings/03/ubc/presentations/Goeckner-pres.pdf
   if (new_right < new_left)
     std::swap(new_right, new_left);
   if (new_bottom < new_top)
     std::swap(new_bottom, new_top);
 
-  return pp::Rect(
-      new_left, new_top, new_right - new_left + 1, new_bottom - new_top + 1);
+  if (!base::IsValueInRangeForNumericType<int32_t>(new_left) ||

[Tom Sepez, 2016/09/27 15:57:17]

new_left is an int (presumably == int32_t for practical purposes), so this would
always be in range?

+      !base::IsValueInRangeForNumericType<int32_t>(new_top)) {
+    return pp::Rect();
+  }
+
+  base::CheckedNumeric<int32_t> new_size_x = new_right;
+  new_size_x -= new_left;
+  new_size_x += 1;
+  base::CheckedNumeric<int32_t> new_size_y = new_bottom;
+  new_size_y -= new_top;
+  new_size_y += 1;
+  if (!new_size_x.IsValid() || !new_size_y.IsValid())
+    return pp::Rect();
+
+  return pp::Rect(new_left, new_top, new_size_x.ValueOrDie(),
+                  new_size_y.ValueOrDie());
 }
 
 PDFiumPage::ScopedLoadCounter::ScopedLoadCounter(PDFiumPage* page)
     : page_(page) {
   page_->loading_count_++;
 }
 
 PDFiumPage::ScopedLoadCounter::~ScopedLoadCounter() {
   page_->loading_count_--;
 }
 
 PDFiumPage::Link::Link() = default;
 
 PDFiumPage::Link::Link(const Link& that) = default;
 
 PDFiumPage::Link::~Link() = default;
 
 }  // namespace chrome_pdf