commit d734d197bb5462a65c37b17594a8c8d07dd79bc1
author tsepez <tsepez@chromium.org> Wed Sep 03 23:17:49 2014
committer Commit bot <commit-bot@chromium.org> Wed Sep 03 23:21:06 2014
tree 68915fea247006ba0c0e9753a0b74d8342af8c97
parent 7e4346c9e4ee487beffe85381eab4d4c08a45434

Avoid OOB memcpy in chrome_pdf::CopyImage.

This is a re-work of palmer's patch at https://codereview.chromium.org/515023002/ which has more context, but comes down to stricter bounds checking.

We also correct an arithmetic bug when copying the image behind a control that is positioned before the origin of the image.

BUG=398384

Review URL: https://codereview.chromium.org/519873002

Cr-Commit-Position: refs/heads/master@{#293213}

pdf/control.cc

diff --git a/pdf/control.cc b/pdf/control.cc
index 12bb7ed1..ed911b6 100644
--- a/pdf/control.cc
+++ b/pdf/control.cc
@@ -53,7 +53,7 @@
     return;
 
   pp::Rect draw_rc = pp::Rect(image_data->size()).Intersect(rect());
-  pp::Rect ctrl_rc = pp::Rect(rect().point() - draw_rc.point(), draw_rc.size());
+  pp::Rect ctrl_rc = pp::Rect(draw_rc.point() - rect().point(), draw_rc.size());
   CopyImage(*image_data, draw_rc, &buffer, ctrl_rc, false);
 
   // Temporary move control to origin (0,0) and draw it into temp buffer.

pdf/draw_utils.cc

diff --git a/pdf/draw_utils.cc b/pdf/draw_utils.cc
index 8bc3ac3..7f999f0 100644
--- a/pdf/draw_utils.cc
+++ b/pdf/draw_utils.cc
@@ -51,6 +51,12 @@
   return static_cast<uint8>((processed / 0xFF) & 0xFF);
 }
 
+inline bool ImageDataContainsRect(const pp::ImageData& image_data,
+                                  const pp::Rect& rect) {
+  return rect.width() >= 0 && rect.height() >= 0 &&
+      pp::Rect(image_data.size()).Contains(rect);
+}
+
 bool AlphaBlend(const pp::ImageData& src, const pp::Rect& src_rc,
                 pp::ImageData* dest, const pp::Point& dest_origin,
                 uint8 alpha_adjustment) {
@@ -145,9 +151,12 @@
 void CopyImage(const pp::ImageData& src, const pp::Rect& src_rc,
                pp::ImageData* dest, const pp::Rect& dest_rc,
                bool stretch) {
-  DCHECK(src_rc.width() <= dest_rc.width() &&
-         src_rc.height() <= dest_rc.height());
-  if (src_rc.IsEmpty())
+  if (src_rc.IsEmpty() || !ImageDataContainsRect(src, src_rc))
+    return;
+
+  pp::Rect stretched_rc(dest_rc.point(),
+                        stretch ? dest_rc.size() : src_rc.size());
+  if (stretched_rc.IsEmpty() || !ImageDataContainsRect(*dest, stretched_rc))
     return;
 
   const uint32_t* src_origin_pixel = src.GetAddr32(src_rc.point());