commit fa70cfe377c1f95bbd5e63063577775a9f2cfb4b
author Hui Yingst <nigi@chromium.org> Fri Feb 10 08:13:54 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Feb 10 08:13:54 2023 +0000
tree de777f3601caef7f47b88a637b44aee07df0f260
parent 28dea1466356878720345480138aa1474dec7c3d

Fix an integer overflow in CFX_Path::GetBoundingBoxForStrokePath()

CL [1] allows a move operation that closes the path to be rendered,
which means points with "move + open" combo are no longer the only
moves to be anticipated in CFX_Path::GetBoundingBoxForStrokePath().
Due to [1], integer overflow is triggered when a "move + close" point
is the first point of path.

This CL makes sure "move + close" points are handled when updating
boundary of stroked path.

[1] https://pdfium.googlesource.com/pdfium/+/2ef13c3534c101c5267e48442b60b21ca8409d03

Bug: chromium:1414373, chromium:1414488
Change-Id: Ie11d1269bf116ac3f6c0290e7287c6e3c1ca4a58
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/103691
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Nigi <nigi@chromium.org>

core/fxge/cfx_path.cpp

diff --git a/core/fxge/cfx_path.cpp b/core/fxge/cfx_path.cpp
index 2e5ccd2..d52c21e 100644
--- a/core/fxge/cfx_path.cpp
+++ b/core/fxge/cfx_path.cpp
@@ -340,9 +340,14 @@
   size_t iMiddlePoint = 0;
   bool bJoin;
   while (iPoint < m_Points.size()) {
-    if (m_Points[iPoint].IsTypeAndOpen(CFX_Path::Point::Type::kMove)) {
-      if (iPoint + 1 == m_Points.size())
+    if (m_Points[iPoint].m_Type == CFX_Path::Point::Type::kMove) {
+      if (iPoint + 1 == m_Points.size()) {
+        if (m_Points[iPoint].m_CloseFigure) {
+          // Update `rect` right away since this is the final point to be drawn.
+          rect.UpdateRect(m_Points[iPoint].m_Point);
+        }
         break;
+      }
 
       iStartPoint = iPoint + 1;
       iEndPoint = iPoint;
@@ -358,7 +363,7 @@
         iPoint += 2;
       }
       if (iPoint == m_Points.size() - 1 ||
-          m_Points[iPoint + 1].IsTypeAndOpen(CFX_Path::Point::Type::kMove)) {
+          m_Points[iPoint + 1].m_Type == CFX_Path::Point::Type::kMove) {
         iStartPoint = iPoint - 1;
         iEndPoint = iPoint;
         bJoin = false;
@@ -380,7 +385,7 @@
       UpdateLineEndPoints(&rect, m_Points[iStartPoint].m_Point,
                           m_Points[iEndPoint].m_Point, half_width);
     }
-    iPoint++;
+    ++iPoint;
   }
   return rect;
 }

core/fxge/cfx_path_unittest.cpp

diff --git a/core/fxge/cfx_path_unittest.cpp b/core/fxge/cfx_path_unittest.cpp
index eebad72..4ccb1c2 100644
--- a/core/fxge/cfx_path_unittest.cpp
+++ b/core/fxge/cfx_path_unittest.cpp
@@ -4,6 +4,7 @@
 
 #include "core/fxge/cfx_path.h"
 
+#include "core/fxcrt/fx_coordinates.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(CFX_Path, BasicTest) {
@@ -371,3 +372,36 @@
   EXPECT_EQ(CFX_PointF(65, 82), path.GetPoint(2));
   EXPECT_EQ(CFX_PointF(65, 82), path.GetPoint(3));
 }
+
+TEST(CFX_Path, GetBoundingBoxForStrokePath) {
+  static constexpr float kLineWidth = 1.0f;
+  static constexpr float kMiterLimit = 1.0f;
+
+  {
+    // Test the case that the first/last point is "move" and it closes the
+    // paths.
+    CFX_Path path;
+    path.AppendPoint({2, 0}, CFX_Path::Point::Type::kMove);
+    path.ClosePath();
+    EXPECT_EQ(CFX_FloatRect(2, 0, 2, 0),
+              path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+  }
+
+  {
+    // Test on a regular rect path.
+    CFX_Path path;
+    path.AppendPoint({2, 0}, CFX_Path::Point::Type::kMove);
+    path.AppendPoint({2, 1}, CFX_Path::Point::Type::kLine);
+    path.AppendPoint({0, 1}, CFX_Path::Point::Type::kLine);
+    path.AppendPoint({0, 0}, CFX_Path::Point::Type::kLine);
+    path.ClosePath();
+    EXPECT_EQ(CFX_FloatRect(-1, -1, 3, 2),
+              path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+
+    // If the final point is "move" and the path remains open, it should not
+    // affect the bounding rect.
+    path.AppendPoint({20, 20}, CFX_Path::Point::Type::kMove);
+    EXPECT_EQ(CFX_FloatRect(-1, -1, 3, 2),
+              path.GetBoundingBoxForStrokePath(kLineWidth, kMiterLimit));
+  }
+}