commit cb391259aefd52f09352d35a1bb5b56c0db6db11
author Ryan Harrison <rharrison@chromium.org> Mon May 07 15:39:45 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Mon May 07 15:39:45 2018 +0000
tree 478239bf85fabd43e37715729e39b6ba6cc29ceb
parent 15406fb41d7aa5ab685005e51ff3cf54acb9a15e

Use checked large integer in ContinueQuickStretch

This existing code has the potential for an integer overflow in it.
When overflow occurs in this function scaling may partially succeed.
This is due to how out of range values are being clamped, which
implicitly swallows the overflow.

This CL changes the calculation to be performed in a 64-bit space and
then attempts to down cast it back to 32-bit space at the end. Because
there are multiple steps it is possible for an intermediate value to
cause an overflow in 32 bit space, but the final value to be valid. If
the downcast is not possible then the stretch operation is failed.

An existing test case has been updated, since it encoded an incorrect
result.

BUG=chromium:839245

Change-Id: I637cc1e2d6c6c2d5394599104f76352c20ead021
Reviewed-on: https://pdfium-review.googlesource.com/32056
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>

core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp

diff --git a/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp b/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp
index 48d7ea3..5103d78 100644
--- a/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp
+++ b/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp
@@ -17,7 +17,7 @@
   FPDF_PAGE page = LoadPage(0);
   ASSERT_TRUE(page);
   ScopedFPDFBitmap bitmap = RenderLoadedPage(page);
-  CompareBitmap(bitmap.get(), 612, 792, "a14d7ee573c1b2456d7bf6b7762823cf");
+  CompareBitmap(bitmap.get(), 612, 792, "1940568c9ba33bac5d0b1ee9558c76b3");
   UnloadPage(page);
 }
 

core/fxcrt/fx_safe_types.h

diff --git a/core/fxcrt/fx_safe_types.h b/core/fxcrt/fx_safe_types.h
index 51eb89c..046a984 100644
--- a/core/fxcrt/fx_safe_types.h
+++ b/core/fxcrt/fx_safe_types.h
@@ -12,6 +12,7 @@
 
 typedef pdfium::base::CheckedNumeric<uint32_t> FX_SAFE_UINT32;
 typedef pdfium::base::CheckedNumeric<int32_t> FX_SAFE_INT32;
+typedef pdfium::base::CheckedNumeric<int64_t> FX_SAFE_INT64;
 typedef pdfium::base::CheckedNumeric<size_t> FX_SAFE_SIZE_T;
 typedef pdfium::base::CheckedNumeric<FX_FILESIZE> FX_SAFE_FILESIZE;
 

core/fxge/dib/cfx_imagestretcher.cpp

diff --git a/core/fxge/dib/cfx_imagestretcher.cpp b/core/fxge/dib/cfx_imagestretcher.cpp
index 9d8e290..1c54e3f 100644
--- a/core/fxge/dib/cfx_imagestretcher.cpp
+++ b/core/fxge/dib/cfx_imagestretcher.cpp
@@ -180,17 +180,28 @@
   int src_height = m_pSource->GetHeight();
   for (; m_LineIndex < result_height; ++m_LineIndex) {
     int dest_y;
-    int src_y;
+    FX_SAFE_INT64 calc_buf;
     if (m_bFlipY) {
       dest_y = result_height - m_LineIndex - 1;
-      src_y = (m_DestHeight - (dest_y + m_ClipRect.top) - 1) * src_height /
-              m_DestHeight;
+      calc_buf = m_DestHeight;
+      calc_buf -= dest_y;
+      calc_buf -= m_ClipRect.top;
+      calc_buf -= 1;
+      calc_buf *= src_height;
+      calc_buf /= m_DestHeight;
     } else {
       dest_y = m_LineIndex;
-      src_y = (dest_y + m_ClipRect.top) * src_height / m_DestHeight;
+      calc_buf = dest_y;
+      calc_buf += m_ClipRect.top;
+      calc_buf *= src_height;
+      calc_buf /= m_DestHeight;
     }
-    src_y = pdfium::clamp(src_y, 0, src_height - 1);
 
+    int src_y;
+    if (!calc_buf.AssignIfValid(&src_y))
+      return false;
+
+    src_y = pdfium::clamp(src_y, 0, src_height - 1);
     if (m_pSource->SkipToScanline(src_y, pPause))
       return true;