commit ea545a62547faa4c249fe796d897ff30c1207d59
author Lei Zhang <thestig@chromium.org> Wed Nov 06 22:12:00 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Nov 06 22:12:00 2019 +0000
tree c1670065996c714cfc62430b6a1de072f0ad4e40
parent 5623992135c922ef22d632b22511f56145aa87c3

Avoid crashes in FPDFText_GetCharBox().

Make sure the out parameters are not nullptr before dereferencing them.
Add tests to verify this. Do the same for FPDFText_GetLooseCharBox().

Also use more interesting sentinel values rather than 0 for the purpose
of checking false return values mean the out parameters are unchanged.

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

fpdfsdk/fpdf_text.cpp

diff --git a/fpdfsdk/fpdf_text.cpp b/fpdfsdk/fpdf_text.cpp
index 1387473..a7f97dc 100644
--- a/fpdfsdk/fpdf_text.cpp
+++ b/fpdfsdk/fpdf_text.cpp
@@ -223,6 +223,9 @@
                                                         double* right,
                                                         double* bottom,
                                                         double* top) {
+  if (!left || !right || !bottom || !top)
+    return false;
+
   CPDF_TextPage* textpage = GetTextPageForValidIndex(text_page, index);
   if (!textpage)
     return false;
@@ -243,6 +246,9 @@
                          double* right,
                          double* bottom,
                          double* top) {
+  if (!left || !right || !bottom || !top)
+    return false;
+
   CPDF_TextPage* textpage = GetTextPageForValidIndex(text_page, index);
   if (!textpage)
     return false;

fpdfsdk/fpdf_text_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_text_embeddertest.cpp b/fpdfsdk/fpdf_text_embeddertest.cpp
index e3b8216..7cbd27a 100644
--- a/fpdfsdk/fpdf_text_embeddertest.cpp
+++ b/fpdfsdk/fpdf_text_embeddertest.cpp
@@ -91,25 +91,33 @@
   EXPECT_EQ(12.0, FPDFText_GetFontSize(textpage, 0));
   EXPECT_EQ(16.0, FPDFText_GetFontSize(textpage, 15));
 
-  double left = 0.0;
-  double right = 0.0;
-  double bottom = 0.0;
-  double top = 0.0;
+  double left = 1.0;
+  double right = 2.0;
+  double bottom = 3.0;
+  double top = 4.0;
   EXPECT_FALSE(FPDFText_GetCharBox(nullptr, 4, &left, &right, &bottom, &top));
-  EXPECT_DOUBLE_EQ(0.0, left);
-  EXPECT_DOUBLE_EQ(0.0, right);
-  EXPECT_DOUBLE_EQ(0.0, bottom);
-  EXPECT_DOUBLE_EQ(0.0, top);
+  EXPECT_DOUBLE_EQ(1.0, left);
+  EXPECT_DOUBLE_EQ(2.0, right);
+  EXPECT_DOUBLE_EQ(3.0, bottom);
+  EXPECT_DOUBLE_EQ(4.0, top);
   EXPECT_FALSE(FPDFText_GetCharBox(textpage, -1, &left, &right, &bottom, &top));
-  EXPECT_DOUBLE_EQ(0.0, left);
-  EXPECT_DOUBLE_EQ(0.0, right);
-  EXPECT_DOUBLE_EQ(0.0, bottom);
-  EXPECT_DOUBLE_EQ(0.0, top);
+  EXPECT_DOUBLE_EQ(1.0, left);
+  EXPECT_DOUBLE_EQ(2.0, right);
+  EXPECT_DOUBLE_EQ(3.0, bottom);
+  EXPECT_DOUBLE_EQ(4.0, top);
   EXPECT_FALSE(FPDFText_GetCharBox(textpage, 55, &left, &right, &bottom, &top));
-  EXPECT_DOUBLE_EQ(0.0, left);
-  EXPECT_DOUBLE_EQ(0.0, right);
-  EXPECT_DOUBLE_EQ(0.0, bottom);
-  EXPECT_DOUBLE_EQ(0.0, top);
+  EXPECT_DOUBLE_EQ(1.0, left);
+  EXPECT_DOUBLE_EQ(2.0, right);
+  EXPECT_DOUBLE_EQ(3.0, bottom);
+  EXPECT_DOUBLE_EQ(4.0, top);
+  EXPECT_FALSE(
+      FPDFText_GetCharBox(textpage, 4, nullptr, &right, &bottom, &top));
+  EXPECT_FALSE(FPDFText_GetCharBox(textpage, 4, &left, nullptr, &bottom, &top));
+  EXPECT_FALSE(FPDFText_GetCharBox(textpage, 4, &left, &right, nullptr, &top));
+  EXPECT_FALSE(
+      FPDFText_GetCharBox(textpage, 4, &left, &right, &bottom, nullptr));
+  EXPECT_FALSE(
+      FPDFText_GetCharBox(textpage, 4, nullptr, nullptr, nullptr, nullptr));
 
   EXPECT_TRUE(FPDFText_GetCharBox(textpage, 4, &left, &right, &bottom, &top));
   EXPECT_NEAR(41.071, left, 0.001);
@@ -117,6 +125,39 @@
   EXPECT_NEAR(49.844, bottom, 0.001);
   EXPECT_NEAR(55.520, top, 0.001);
 
+  left = 4.0;
+  right = 3.0;
+  bottom = 2.0;
+  top = 1.0;
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(nullptr, 4, &left, &right, &bottom, &top));
+  EXPECT_DOUBLE_EQ(4.0, left);
+  EXPECT_DOUBLE_EQ(3.0, right);
+  EXPECT_DOUBLE_EQ(2.0, bottom);
+  EXPECT_DOUBLE_EQ(1.0, top);
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, -1, &left, &right, &bottom, &top));
+  EXPECT_DOUBLE_EQ(4.0, left);
+  EXPECT_DOUBLE_EQ(3.0, right);
+  EXPECT_DOUBLE_EQ(2.0, bottom);
+  EXPECT_DOUBLE_EQ(1.0, top);
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, 55, &left, &right, &bottom, &top));
+  EXPECT_DOUBLE_EQ(4.0, left);
+  EXPECT_DOUBLE_EQ(3.0, right);
+  EXPECT_DOUBLE_EQ(2.0, bottom);
+  EXPECT_DOUBLE_EQ(1.0, top);
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, 4, nullptr, &right, &bottom, &top));
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, 4, &left, nullptr, &bottom, &top));
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, 4, &left, &right, nullptr, &top));
+  EXPECT_FALSE(
+      FPDFText_GetLooseCharBox(textpage, 4, &left, &right, &bottom, nullptr));
+  EXPECT_FALSE(FPDFText_GetLooseCharBox(textpage, 4, nullptr, nullptr, nullptr,
+                                        nullptr));
+
   EXPECT_TRUE(
       FPDFText_GetLooseCharBox(textpage, 4, &left, &right, &bottom, &top));
   EXPECT_NEAR(40.664, left, 0.001);