commit a163e26c830518c9a92c09c7f6011f5b517b20c1
author Lei Zhang <thestig@chromium.org> Fri Sep 11 00:25:14 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Fri Sep 11 00:25:14 2020 +0000
tree 0d758fe21b5f4912026b40392cc583f190368d4c
parent 2648259d285b086196cb25b81710c5345cb04261

Use CheckedNumerics to calculate size in CalculatePitchAndSize().

In CFX_DIBitmap::CalculatePitchAndSize(), instead of manually avoiding
integer overflow when calculating the pitch, use CheckedNumerics instead
by instantiating a FX_SAFE_UINT32.

This raises the limit for the maximum possible size.

Bug: chromium:1124660
Change-Id: Ib9ec3f68db1d8e78c7d893758f0ec1ceaf6cfe28
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73398
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

core/fxge/dib/cfx_dibitmap.cpp

diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp
index 8a9c56e..587dac3 100644
--- a/core/fxge/dib/cfx_dibitmap.cpp
+++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -866,11 +866,12 @@
     actual_pitch = safe_pitch.ValueOrDie();
   }
 
-  if ((1 << 30) / actual_pitch < static_cast<uint32_t>(height))
+  FX_SAFE_UINT32 safe_size = actual_pitch;
+  safe_size *= height;
+  if (!safe_size.IsValid())
     return pdfium::nullopt;
 
-  uint32_t size = actual_pitch * static_cast<uint32_t>(height);
-  return PitchAndSize{actual_pitch, size};
+  return PitchAndSize{actual_pitch, safe_size.ValueOrDie()};
 }
 
 bool CFX_DIBitmap::CompositeBitmap(int dest_left,

core/fxge/dib/cfx_dibitmap_unittest.cpp

diff --git a/core/fxge/dib/cfx_dibitmap_unittest.cpp b/core/fxge/dib/cfx_dibitmap_unittest.cpp
index 507fdbe..08d5ad9 100644
--- a/core/fxge/dib/cfx_dibitmap_unittest.cpp
+++ b/core/fxge/dib/cfx_dibitmap_unittest.cpp
@@ -68,8 +68,8 @@
       CFX_DIBitmap::CalculatePitchAndSize(4194304, 1024, FXDIB_8bppRgb, 0));
 
   // Overflow cases with provided pitch.
-  EXPECT_FALSE(CFX_DIBitmap::CalculatePitchAndSize(1073741825, 1, FXDIB_Argb,
-                                                   1073741825));
+  EXPECT_FALSE(CFX_DIBitmap::CalculatePitchAndSize(2147484000u, 1, FXDIB_Argb,
+                                                   2147484000u));
   EXPECT_FALSE(
       CFX_DIBitmap::CalculatePitchAndSize(1048576, 1024, FXDIB_Argb, 4194304));
   EXPECT_FALSE(CFX_DIBitmap::CalculatePitchAndSize(4194304, 1024, FXDIB_8bppRgb,
@@ -79,18 +79,18 @@
 TEST(CFX_DIBitmap, CalculatePitchAndSizeBoundary) {
   // Test boundary condition for pitch overflow.
   Optional<CFX_DIBitmap::PitchAndSize> result =
-      CFX_DIBitmap::CalculatePitchAndSize(268435456, 4, FXDIB_8bppRgb, 0);
+      CFX_DIBitmap::CalculatePitchAndSize(536870908, 4, FXDIB_8bppRgb, 0);
   ASSERT_TRUE(result);
-  EXPECT_EQ(268435456u, result.value().pitch);
-  EXPECT_EQ(1073741824u, result.value().size);
+  EXPECT_EQ(536870908u, result.value().pitch);
+  EXPECT_EQ(2147483632u, result.value().size);
   EXPECT_FALSE(
-      CFX_DIBitmap::CalculatePitchAndSize(268435457, 4, FXDIB_8bppRgb, 0));
+      CFX_DIBitmap::CalculatePitchAndSize(536870909, 4, FXDIB_8bppRgb, 0));
 
   // Test boundary condition for size overflow.
-  result = CFX_DIBitmap::CalculatePitchAndSize(17043520, 63, FXDIB_8bppRgb, 0);
+  result = CFX_DIBitmap::CalculatePitchAndSize(68174084, 63, FXDIB_8bppRgb, 0);
   ASSERT_TRUE(result);
-  EXPECT_EQ(17043520u, result.value().pitch);
-  EXPECT_EQ(1073741760u, result.value().size);
+  EXPECT_EQ(68174084u, result.value().pitch);
+  EXPECT_EQ(4294967292u, result.value().size);
   EXPECT_FALSE(
-      CFX_DIBitmap::CalculatePitchAndSize(17043521, 63, FXDIB_8bppRgb, 0));
+      CFX_DIBitmap::CalculatePitchAndSize(68174085, 63, FXDIB_8bppRgb, 0));
 }