From 29845c187e0b0d13fe6435b6259a15cad2d22fd7 Mon Sep 17 00:00:00 2001
From: Conor Bronsdon <120674402+conorbronsdon@users.noreply.github.com>
Date: Sun, 5 Jul 2026 23:11:19 -0700
Subject: [PATCH] fix: harden against confirmed review findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make _strip_tags quote-aware so a raw `>` inside a quoted attribute of
raw HTML in an image description cannot terminate the tag early and break
out of the generated alt="…" attribute (injection-prone unbalanced HTML).
Add a regression test covering double/single-quoted attributes and a
nested <img>.

Document in SECURITY.md that output is NOT sanitized: raw HTML and
javascript:/data: URLs pass through verbatim per CommonMark, so rendering
untrusted markdown into a live page is a stored-XSS risk that callers must
sanitize themselves.

Co-Authored-By: Claude <noreply@anthropic.com>
---
 SECURITY.md              | 24 ++++++++++++++++++++++--
 src/markdown/inline.mojo | 25 +++++++++++++++++++++----
 test/test_markdown.mojo  | 22 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/SECURITY.md b/SECURITY.md
index 0a0c920..fa64ba6 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -2,8 +2,28 @@
 
 mojo-markdown is a pure-Mojo CommonMark parser and HTML renderer with no
 network access, no authentication, and no secrets handling — it reads
-markdown text and returns rendered HTML or an AST. The main risk
-surface is malformed or adversarial input causing a crash or hang.
+markdown text and returns rendered HTML or an AST.
+
+## Output is NOT sanitized
+
+Per the [CommonMark spec](https://spec.commonmark.org/), raw inline and
+block HTML, as well as `javascript:`, `data:`, and other arbitrary URL
+schemes in links and images, pass through to the output **verbatim**.
+mojo-markdown does **not** sanitize its output. Rendering untrusted
+markdown and inserting the result into a live page is a stored-XSS risk:
+an author can embed `<script>`, event-handler attributes, or a
+`javascript:` link and it will be emitted unchanged.
+
+If you render markdown from untrusted sources, run the HTML through a
+dedicated sanitizer/allow-list (e.g. an equivalent of DOMPurify) before
+displaying it. This library deliberately preserves spec-compliant
+passthrough rather than sanitizing, so sanitization is the caller's
+responsibility.
+
+## Reporting crashes and hangs
+
+Beyond the passthrough behavior above, the remaining risk surface is
+malformed or adversarial input causing a crash or hang.
 
 If you find an input that crashes, hangs, or otherwise misbehaves in a
 way that looks security-relevant (e.g. out-of-bounds access, unbounded
diff --git a/src/markdown/inline.mojo b/src/markdown/inline.mojo
index 6f814a1..021d745 100644
--- a/src/markdown/inline.mojo
+++ b/src/markdown/inline.mojo
@@ -398,9 +398,12 @@ def _parse_inline_suffix(s: String, lparen: Int) -> _LinkSuffix:
 def _strip_tags(html: String) -> String:
     """Remove HTML tags, leaving text content (for image alt attributes).
 
-    Operates on already-escaped inline HTML, so `<`/`>` only appear as tag
-    delimiters and attribute values never contain a raw `>`. A nested
-    `<img>` contributes its `alt` text rather than vanishing.
+    Operates on inline HTML. Raw inline HTML passes through verbatim, so an
+    attribute value *can* contain a raw `>` (e.g. `<span title="a>b">`). The
+    tag scan is therefore quote-aware: it only treats a `>` outside a quoted
+    attribute as the tag terminator, so such a `>` cannot break out of the
+    surrounding `alt="…"` attribute. A nested `<img>` contributes its `alt`
+    text rather than vanishing.
     """
     var bytes = html.as_bytes()
     var n = len(bytes)
@@ -409,8 +412,22 @@ def _strip_tags(html: String) -> String:
     while i < n:
         var b = bytes[i]
         if b == LT:
+            # Scan to the tag's closing `>`, skipping over quoted attribute
+            # values so a raw `>` inside an attribute does not end the tag
+            # early.
             var j = i + 1
-            while j < n and bytes[j] != GT:
+            var in_quote = False
+            var quote_ch = UInt8(0)
+            while j < n:
+                var c = bytes[j]
+                if in_quote:
+                    if c == quote_ch:
+                        in_quote = False
+                elif c == QUOTE or c == SQUOTE:
+                    in_quote = True
+                    quote_ch = c
+                elif c == GT:
+                    break
                 j += 1
             # A nested image keeps its alt text.
             if _match_bytes(bytes, i + 1, "img ") or _match_bytes(
diff --git a/test/test_markdown.mojo b/test/test_markdown.mojo
index 7df8969..30ea8b8 100644
--- a/test/test_markdown.mojo
+++ b/test/test_markdown.mojo
@@ -194,6 +194,28 @@ def test_image_alt_is_plain_text() raises:
     )
 
 
+def test_image_alt_raw_html_gt_no_breakout() raises:
+    # A raw `>` inside a quoted attribute of raw HTML in the image
+    # description must not terminate the tag early and break out of the
+    # generated alt="…" attribute. Raw HTML contributes no alt text, so
+    # only the trailing literal `x` survives.
+    assert_equal(
+        render_html(String('![<span title="a>b">x](/i.png)')),
+        '<p><img src="/i.png" alt="x" /></p>\n',
+    )
+    # Single-quoted attribute value is handled the same way.
+    assert_equal(
+        render_html(String("![<span title='a>b'>x](/i.png)")),
+        '<p><img src="/i.png" alt="x" /></p>\n',
+    )
+    # A nested <img> still contributes its alt text, even when a later
+    # attribute contains a raw `>`.
+    assert_equal(
+        render_html(String('![<img alt="D" title="a>b">z](/i.png)')),
+        '<p><img src="/i.png" alt="Dz" /></p>\n',
+    )
+
+
 def test_autolink() raises:
     assert_equal(
         render_html("<https://example.com>"),