[PATCH v3 0/1] MR3398: gdiplus: Prevent infinite loops due to floating point inaccuracy

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52492

--
  v3: gdiplus: Prevent infinite loops due to floating point inaccuracy

https://gitlab.winehq.org/wine/wine/-/merge_requests/3398

[PATCH v3 1/1] gdiplus: Prevent infinite loops due to floating point inaccuracy

[Fabian Maurer]

From: Fabian Maurer

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52492
---
 dlls/gdiplus/graphicspath.c       | 12 +++++-----
 dlls/gdiplus/tests/graphicspath.c | 37 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c
index 06e17d4b98a..545aaa8441e 100644
--- a/dlls/gdiplus/graphicspath.c
+++ b/dlls/gdiplus/graphicspath.c
@@ -183,14 +183,16 @@ static BOOL flatten_bezier(path_list_node_t *start, REAL x2, REAL y2, REAL x3, R
         mp[2].X = (mp[1].X + mp[3].X) / 2.0;
         mp[2].Y = (mp[1].Y + mp[3].Y) / 2.0;

-        /* Is one pair of the new control points equal to the old control points? */
-        if ((x2 == mp[0].X && y2 == mp[0].Y && x3 == mp[1].X && y3 == mp[1].Y) ||
-            (x2 == mp[3].X && y2 == mp[3].Y && x3 == mp[4].X && y3 == mp[4].Y))
-            continue;
-
         pt = end->pt;
         pt_st = start->pt;

+        /* Test for closely spaced points that don't need to be flattened
+         * Also avoids limited-precision errors in flatness check
+         */
+        if((fabs(pt.X - mp[2].X) + fabs(pt.Y - mp[2].Y) +
+            fabs(pt_st.X - mp[2].X) + fabs(pt_st.Y - mp[2].Y) ) <= flatness * 0.5)
+            continue;
+
         /* check flatness as a half of distance between middle point and a linearized path
          * formula for distance point from line for point (x0, y0) and line (x1, y1) (x2, y2)
          * is defined as (area_triangle / distance_start_end):
diff --git a/dlls/gdiplus/tests/graphicspath.c b/dlls/gdiplus/tests/graphicspath.c
index 7cbcb51edfc..558eedc147f 100644
--- a/dlls/gdiplus/tests/graphicspath.c
+++ b/dlls/gdiplus/tests/graphicspath.c
@@ -1253,6 +1253,42 @@ static void test_flatten(void)
     GdipDeletePath(path);
 }

+static void test_flatten2(void)
+{
+    GpStatus status;
+    GpPath *path;
+
+    status = GdipCreatePath(0, &path);
+    expect(Ok, status);
+    status = GdipStartPathFigure(path);
+    expect(Ok, status);
+
+    /* path seen in the wild that caused a stack overflow */
+    status = GdipAddPathArc(path, -136.33, 20.00, 786.00, 786.00, -105.00, 30.00);
+    expect(Ok, status);
+    status = GdipAddPathArc(path, 256.67, 413.00, 0.00, 0.00, -75.00, -30.00);
+    expect(Ok, status);
+    status = GdipClosePathFigure(path);
+    expect(Ok, status);
+
+    status = GdipFlattenPath(path, NULL, 1.0);
+    expect(Ok, status);
+
+    /* path seen in the wild that caused a stack overflow */
+    /* same path but redo with the manual points that caused a crash */
+    status = GdipResetPath(path);
+    expect(Ok, status);
+    status = GdipAddPathBezier(path, 154.950806, 33.391144, 221.586075, 15.536285, 291.747314, 15.536285, 358.382568, 33.391144);
+    expect(Ok, status);
+    status = GdipAddPathBezier(path, 256.666809, 412.999512, 256.666718, 412.999481, 256.666656, 412.999481, 256.666565, 412.999512);
+    expect(Ok, status);
+    status = GdipClosePathFigure(path);
+    expect(Ok, status);
+    status = GdipFlattenPath(path, NULL, 1.0);
+
+    GdipDeletePath(path);
+}
+
 static path_test_t widenline_path[] = {
     {5.0, 5.0,   PathPointTypeStart, 0, 0}, /*0*/
     {50.0, 5.0,  PathPointTypeLine,  0, 0}, /*1*/
@@ -1935,6 +1971,7 @@ START_TEST(graphicspath)
     test_widen_cap();
     test_isvisible();
     test_empty_rect();
+    test_flatten2();

GdiplusShutdown(gdiplusToken);
 }

[PATCH v3 1/1] gdiplus: Prevent infinite loops due to floating point inaccuracy

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=135175

Your paranoid android.

=== build (build log) ===

error: patch failed: dlls/gdiplus/graphicspath.c:183
Task: Patch failed to apply

=== debian11 (build log) ===

error: patch failed: dlls/gdiplus/graphicspath.c:183
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/gdiplus/graphicspath.c:183
Task: Patch failed to apply

[Fabian Maurer (＠DarkShadow44)]

On Mon Jul 24 22:20:14 2023 +0000, **** wrote:
> Marvin replied on the mailing list:
> Hi,
> It looks like your patch introduced the new failures shown below.
> Please investigate and fix them before resubmitting your patch.
> If they are not new, fixing them anyway would help a lot. Otherwise
> please ask for the known failures list to be updated.
> The full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=135175
> Your paranoid android.
> === build (build log) ===
> error: patch failed: dlls/gdiplus/graphicspath.c:183
> Task: Patch failed to apply
> === debian11 (build log) ===
> error: patch failed: dlls/gdiplus/graphicspath.c:183
> Task: Patch failed to apply
> === debian11b (build log) ===
> error: patch failed: dlls/gdiplus/graphicspath.c:183
> Task: Patch failed to apply

Seems like the testbot is still checking out the old code, not sure why, but it thinks eac34b9c85e1d6da483eb64094fa0b316d1b01a5 is the HEAD.

[PATCH v3 0/1] MR3398: gdiplus: Prevent infinite loops due to floating point inaccuracy - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.