From 66997f18fc66bc888117c556190beda993ececd2 Mon Sep 17 00:00:00 2001
From: DemchaAV <demchaav@gmail.com>
Date: Sun, 14 Jun 2026 13:07:11 +0100
Subject: [PATCH] fix(svg): reject malformed 'd' that loops forever after a Z
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A Z/z close command consumes no operands, so a stray non-command token
after it (e.g. "M0 0 Z5") made the path scanner re-apply Z indefinitely,
appending a close op every pass until the heap was exhausted — a DoS
reachable from the @Beta SvgPath.parse / SvgIcon public surface on a
single malformed path string.

Add a no-progress guard to the scan loop: if an iteration consumes
neither a command letter nor an operand, fail with the usual
position-carrying IllegalArgumentException instead of spinning.

SvgPathTest gains a regression case wrapped in assertTimeoutPreemptively
so the hang can never silently return.
---
 CHANGELOG.md                                  |  8 ++++++++
 .../compose/document/svg/SvgPathParser.java   | 10 ++++++++++
 .../compose/document/svg/SvgPathTest.java     | 20 +++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5301b9d8d..e8e956fd5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -220,6 +220,14 @@ Entries land here as they merge.
 
 ### Bug fixes
 
+- **SVG path reader no longer hangs on malformed `d` data.** A `Z`/`z`
+  close command (which consumes no operands) followed by a stray
+  non-command token — e.g. `"M0 0 Z5"` — made the scanner loop forever,
+  appending a close op every pass until the heap was exhausted. A single
+  malformed or hostile path string could therefore DoS the `@Beta`
+  `SvgPath.parse` / `SvgIcon` reader. The scanner now fails fast with the
+  usual position-carrying `IllegalArgumentException` when an iteration
+  consumes neither a command nor an operand.
 - **`BEHIND_CONTENT` watermarks no longer wash out the page.** The PDF
   watermark renderer set its low-opacity graphics state in a *prepended*
   content stream without a save/restore pair; PDFBox's `resetContext` only
diff --git a/src/main/java/com/demcha/compose/document/svg/SvgPathParser.java b/src/main/java/com/demcha/compose/document/svg/SvgPathParser.java
index 34ca597d7..ec7399761 100644
--- a/src/main/java/com/demcha/compose/document/svg/SvgPathParser.java
+++ b/src/main/java/com/demcha/compose/document/svg/SvgPathParser.java
@@ -54,6 +54,7 @@ List<double[]> parse() {
                 break;
             }
             char c = d.charAt(pos);
+            int loopStart = pos;
             if (isCommand(c)) {
                 cmd = c;
                 pos++;
@@ -68,6 +69,15 @@ List<double[]> parse() {
                 }
             }
             apply(cmd);
+            if (pos == loopStart) {
+                // No command letter and no operand was consumed this iteration.
+                // This only happens when an operand-less command (Z/z) is
+                // implicitly repeated by a stray non-command token, e.g.
+                // "M0 0 Z5": apply(Z) consumes nothing, so without this guard
+                // the loop would spin forever, appending a close op each pass
+                // until the heap is exhausted. Fail loudly instead of hanging.
+                throw fail("a command letter");
+            }
         }
         return ops;
     }
diff --git a/src/test/java/com/demcha/compose/document/svg/SvgPathTest.java b/src/test/java/com/demcha/compose/document/svg/SvgPathTest.java
index a8aff6fc3..039910863 100644
--- a/src/test/java/com/demcha/compose/document/svg/SvgPathTest.java
+++ b/src/test/java/com/demcha/compose/document/svg/SvgPathTest.java
@@ -5,8 +5,10 @@
 import com.demcha.compose.document.style.DocumentPathSegment.CubicTo;
 import com.demcha.compose.document.style.DocumentPathSegment.LineTo;
 import com.demcha.compose.document.style.DocumentPathSegment.MoveTo;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.time.Duration;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -197,4 +199,22 @@ void syntaxErrorsCarryThePosition() {
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("viewBox");
     }
+
+    @Test
+    void strayTokenAfterCloseIsRejectedAndDoesNotHang() {
+        // Regression: an operand-less Z/z followed by a non-command token used
+        // to spin forever (the close op consumes no characters, so the scanner
+        // never advanced), appending a close op per iteration until OOM. A
+        // single malformed/hostile 'd' string would DoS the @Beta reader.
+        // Each call must fail fast; the assertTimeout pins that it cannot hang.
+        Assertions.assertTimeoutPreemptively(
+                Duration.ofSeconds(2), () -> {
+                    assertThatThrownBy(() -> SvgPath.parse("M0 0 Z5"))
+                            .isInstanceOf(IllegalArgumentException.class)
+                            .hasMessageContaining("position");
+                    assertThatThrownBy(() -> SvgPath.parse("M0 0 z9 9"))
+                            .isInstanceOf(IllegalArgumentException.class)
+                            .hasMessageContaining("position");
+                });
+    }
 }