From cb391259aefd52f09352d35a1bb5b56c0db6db11 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Mon, 7 May 2018 15:39:45 +0000 Subject: 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 Commit-Queue: Ryan Harrison --- .../render/fpdf_render_loadimage_embeddertest.cpp | 2 +- core/fxcrt/fx_safe_types.h | 1 + core/fxge/dib/cfx_imagestretcher.cpp | 21 ++++++++++++++++----- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp b/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp index 48d7ea3399..5103d787bd 100644 --- a/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp +++ b/core/fpdfapi/render/fpdf_render_loadimage_embeddertest.cpp @@ -17,7 +17,7 @@ TEST_F(FPDFRenderLoadImageEmbeddertest, Bug_554151) { 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); } diff --git a/core/fxcrt/fx_safe_types.h b/core/fxcrt/fx_safe_types.h index 51eb89cf76..046a98479e 100644 --- a/core/fxcrt/fx_safe_types.h +++ b/core/fxcrt/fx_safe_types.h @@ -12,6 +12,7 @@ typedef pdfium::base::CheckedNumeric FX_SAFE_UINT32; typedef pdfium::base::CheckedNumeric FX_SAFE_INT32; +typedef pdfium::base::CheckedNumeric FX_SAFE_INT64; typedef pdfium::base::CheckedNumeric FX_SAFE_SIZE_T; typedef pdfium::base::CheckedNumeric FX_SAFE_FILESIZE; diff --git a/core/fxge/dib/cfx_imagestretcher.cpp b/core/fxge/dib/cfx_imagestretcher.cpp index 9d8e290b43..1c54e3fdd0 100644 --- a/core/fxge/dib/cfx_imagestretcher.cpp +++ b/core/fxge/dib/cfx_imagestretcher.cpp @@ -180,17 +180,28 @@ bool CFX_ImageStretcher::ContinueQuickStretch(PauseIndicatorIface* pPause) { 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; -- cgit v1.2.3