commit 0c83d12c1af12cf67e8bab47312ff76a84a6afbf
author Lei Zhang <thestig@chromium.org> Fri Aug 02 18:25:07 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Aug 02 18:25:07 2024 +0000
tree e33a413f0a2607daaf674c344a48168695f13c51
parent e3585e9545292684c0b787b8ca81b83d66bb903f

Check for integer overflows in FPDFBitmap_FillRect()

Better validate the input using safe math calculation to avoid undefined
behavior. Add an embedder test to help show these cases are handled
correctly.

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

fpdfsdk/fpdf_view.cpp

diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index f978330..1d24fa0 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -967,7 +967,19 @@
   }
   CHECK(!pBitmap->IsPremultiplied());
 
-  FX_RECT fill_rect(left, top, left + width, top + height);
+  FX_SAFE_INT32 right = left;
+  right += width;
+  if (!right.IsValid()) {
+    return;
+  }
+
+  FX_SAFE_INT32 bottom = top;
+  bottom += height;
+  if (!bottom.IsValid()) {
+    return;
+  }
+
+  FX_RECT fill_rect(left, top, right.ValueOrDie(), bottom.ValueOrDie());
 
   if (!pBitmap->IsAlphaFormat()) {
     color |= 0xFF000000;

fpdfsdk/fpdf_view_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 74f0d68..200f307 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -2196,3 +2196,32 @@
   TestRenderPageBitmapWithInternalMemory(page.get(), FPDFBitmap_Gray,
                                          gray_checksum);
 }
+
+TEST_F(FPDFViewEmbedderTest, BadFillRectInput) {
+  constexpr int kWidth = 200;
+  constexpr int kHeight = 200;
+  constexpr char kExpectedChecksum[] = "acc736435c9f84aa82941ba561bc5dbc";
+  ScopedFPDFBitmap bitmap(FPDFBitmap_Create(200, 200, /*alpha=*/true));
+  FPDFBitmap_FillRect(bitmap.get(), /*left=*/0, /*top=*/0, /*width=*/kWidth,
+                      /*height=*/kHeight, 0xFFFF0000);
+  EXPECT_EQ(kExpectedChecksum, HashBitmap(bitmap.get()));
+
+  // Empty rect dimensions is a no-op.
+  FPDFBitmap_FillRect(bitmap.get(), /*left=*/0, /*top=*/0, /*width=*/0,
+                      /*height=*/0, 0xFF0000FF);
+  EXPECT_EQ(kExpectedChecksum, HashBitmap(bitmap.get()));
+
+  // Rect dimension overflows are also no-ops.
+  FPDFBitmap_FillRect(bitmap.get(), /*left=*/std::numeric_limits<int>::max(),
+                      /*top=*/0, /*width=*/std::numeric_limits<int>::max(),
+                      /*height=*/kHeight, 0xFF0000FF);
+  EXPECT_EQ(kExpectedChecksum, HashBitmap(bitmap.get()));
+
+  FPDFBitmap_FillRect(bitmap.get(), /*left=*/0,
+                      /*top=*/std::numeric_limits<int>::max(), /*width=*/kWidth,
+                      /*height=*/std::numeric_limits<int>::max(), 0xFF0000FF);
+  EXPECT_EQ(kExpectedChecksum, HashBitmap(bitmap.get()));
+
+  // Make sure null bitmap handle does not trigger a crash.
+  FPDFBitmap_FillRect(nullptr, 0, 0, kWidth, kHeight, 0xFF0000FF);
+}