From a903c2f242856b02380f9357c03ad758395f5ad6 Mon Sep 17 00:00:00 2001 From: aadrian Date: Wed, 16 Sep 2026 21:55:26 +0200 Subject: [PATCH 1/2] Fix ReDoS, stack-overflow, and CSS-injection in Markdown rendering, with regression tests. --- .../io/github/gitbucket/markedj/Grammer.java | 10 +++- .../io/github/gitbucket/markedj/Lexer.java | 24 ++++++++ .../io/github/gitbucket/markedj/Options.java | 26 ++++++++- .../github/gitbucket/markedj/MarkedTest.java | 56 +++++++++++++++++++ 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/github/gitbucket/markedj/Grammer.java b/src/main/java/io/github/gitbucket/markedj/Grammer.java index a9945d1..45006a8 100644 --- a/src/main/java/io/github/gitbucket/markedj/Grammer.java +++ b/src/main/java/io/github/gitbucket/markedj/Grammer.java @@ -76,7 +76,13 @@ private static String removeLineStart(String regex){ public static Map INLINE_BREAKS_RULES = new HashMap<>(); public static String INLINE_ESCAPE = "^\\\\([\\\\`*{}\\[\\]()#+\\-.!_>])"; - public static String INLINE_TEXT = "^[\\s\\S]+?(?=[\\\\ rules = null; + private int depth = 0; public Lexer(Options options){ this.options = options; @@ -39,6 +48,21 @@ public LexerResult lex(String src){ } protected void token(String src, boolean top, boolean bq, LexerContext context){ + depth++; + try { + if(depth > MAX_NESTING_DEPTH){ + if(!src.isEmpty()){ + context.pushToken(new ParagraphToken(src)); + } + return; + } + tokenInternal(src, top, bq, context); + } finally { + depth--; + } + } + + private void tokenInternal(String src, boolean top, boolean bq, LexerContext context){ while(src.length() > 0){ // newline { diff --git a/src/main/java/io/github/gitbucket/markedj/Options.java b/src/main/java/io/github/gitbucket/markedj/Options.java index a7e8e98..eff649b 100644 --- a/src/main/java/io/github/gitbucket/markedj/Options.java +++ b/src/main/java/io/github/gitbucket/markedj/Options.java @@ -3,6 +3,9 @@ import io.github.gitbucket.markedj.extension.Extension; import java.util.ArrayList; import java.util.List; +import java.util.regex.Pattern; +import org.jsoup.nodes.Attribute; +import org.jsoup.nodes.Element; import org.jsoup.safety.Safelist; public class Options { @@ -13,7 +16,28 @@ public class Options { private boolean sanitize = false; private String langPrefix = "lang-"; private String headerPrefix = ""; - private Safelist safelist = new Safelist() + + // jsoup only checks whether "style" may exist on a tag, never what CSS it contains, + // so allowing it unconditionally would let raw HTML in markdown carry arbitrary CSS + // (clickjacking overlays via position:fixed, data exfiltration via attribute + // selectors, etc). The only legitimate producer of "style" in this library is + // Renderer.tablecell()'s table-column alignment, so that exact shape is all that's + // allowed through; everything else is stripped. + private static final Pattern SAFE_STYLE_VALUE = + Pattern.compile("^text-align: (left|right|center)$"); + + private Safelist safelist = new Safelist() { + @Override + public boolean isSafeAttribute(String tagName, Element el, Attribute attr) { + if (!super.isSafeAttribute(tagName, el, attr)) { + return false; + } + if ("style".equals(attr.getKey())) { + return SAFE_STYLE_VALUE.matcher(attr.getValue()).matches(); + } + return true; + } + } .addTags( "a", "b", "blockquote", "br", "caption", "cite", "code", "col", "colgroup", "dd", "div", "dl", "dt", "em", "h1", "h2", "h3", "h4", "h5", "h6", diff --git a/src/test/java/io/github/gitbucket/markedj/MarkedTest.java b/src/test/java/io/github/gitbucket/markedj/MarkedTest.java index f14a857..e021e7d 100644 --- a/src/test/java/io/github/gitbucket/markedj/MarkedTest.java +++ b/src/test/java/io/github/gitbucket/markedj/MarkedTest.java @@ -273,6 +273,31 @@ public void testHardLineBreakWithSpaces() { " Line 2

", result); } + // Options' default Safelist allows the "style" attribute on every tag (":all"), + // but jsoup only checks whether an attribute may exist -- it never validates CSS + // *content*. Arbitrary attacker-controlled style is a real primitive (clickjacking + // overlays via `position:fixed`, data exfiltration via CSS attribute selectors), + // reachable via any raw HTML in markdown (sanitize=false is also the default). + // The only legitimate producer of `style` in this library is + // Renderer.tablecell()'s "text-align: left|right|center", so nothing but that + // exact shape should ever survive. + @Test + public void testStyleAttributeRejectsArbitraryCss() throws Exception { + String result = Marked.marked( + "
clickjack
"); + assertEquals("
\n clickjack\n
", result); + } + + // The fix for the above must not break the one legitimate use of `style`: + // column alignment on tables. + @Test + public void testStyleAttributeAllowsTableAlignment() throws Exception { + String result = Marked.marked("|A|B|\n|--:|:--|\n|1|2|"); + assertTrue(result.contains("style=\"text-align: right\"")); + assertTrue(result.contains("style=\"text-align: left\"")); + } + @Test public void testHardLineBreakWithBackslash() { String result = Marked.marked("Line 1\\\n" + @@ -280,4 +305,35 @@ public void testHardLineBreakWithBackslash() { assertEquals("

Line 1
\n" + " Line 2

", result); } + + // Reproduces a quadratic-time blowup in the inline "text" rule (Grammer.INLINE_TEXT). + // A single character followed by a long run of trailing spaces and no terminating + // newline forces the lazy `[\s\S]+?` loop to re-probe the ` {2,}\n` lookahead at + // every one of the ~50,000 positions, and each probe rescans the remaining space + // run looking for a `\n` that never comes -- O(n^2) total work from one paragraph. + // This is trivially reachable via any GitBucket issue/PR/comment/wiki body. + @Test(timeout = 2000) + public void testLongTrailingSpacesDoesNotHang() { + StringBuilder sb = new StringBuilder("x"); + for (int i = 0; i < 50_000; i++) { + sb.append(' '); + } + Marked.marked(sb.toString()); + } + + // Lexer.token() recurses once per nesting level of "> " to handle nested + // blockquotes, with no depth limit. A few thousand nesting levels (trivially + // reachable via any GitBucket issue/PR/comment/wiki body) blow the JVM call + // stack with an uncaught StackOverflowError, which -- unlike a checked + // exception -- will not be caught by ordinary `catch (Exception e)` error + // handling around the rendering call, and can crash the calling thread. + @Test + public void testDeeplyNestedBlockquoteDoesNotOverflow() { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 5_000; i++) { + sb.append("> "); + } + sb.append("hello"); + Marked.marked(sb.toString()); + } } From a033a70d11f4060269e270b5d118c540a7a0b434 Mon Sep 17 00:00:00 2001 From: aadrian Date: Thu, 17 Sep 2026 06:47:27 +0200 Subject: [PATCH 2/2] Fix ReDoS still reachable via breaks=true, per reviewer feedback. --- .../java/io/github/gitbucket/markedj/Grammer.java | 5 ++++- .../io/github/gitbucket/markedj/MarkedTest.java | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/github/gitbucket/markedj/Grammer.java b/src/main/java/io/github/gitbucket/markedj/Grammer.java index 45006a8..c8a2740 100644 --- a/src/main/java/io/github/gitbucket/markedj/Grammer.java +++ b/src/main/java/io/github/gitbucket/markedj/Grammer.java @@ -108,7 +108,10 @@ private static String removeLineStart(String regex){ INLINE_BREAKS_RULES.putAll(INLINE_GFM_RULES); INLINE_BREAKS_RULES.put("br", new FindFirstRule(INLINE_BR.replace("{2,}", "*"))); - INLINE_BREAKS_RULES.put("text", new FindFirstRule(INLINE_TEXT.replace("]|", "~]|").replace("|", "|https?://|").replace("{2,20}", "*"))); + // Use "{0,20}" here, not "*": breaks mode only needs to lower the minimum from + // 2 spaces to 0 (any single newline is a break), not drop the upper bound that + // keeps this lookahead from being the same O(n^2) trap described above. + INLINE_BREAKS_RULES.put("text", new FindFirstRule(INLINE_TEXT.replace("]|", "~]|").replace("|", "|https?://|").replace("{2,20}", "{0,20}"))); } } diff --git a/src/test/java/io/github/gitbucket/markedj/MarkedTest.java b/src/test/java/io/github/gitbucket/markedj/MarkedTest.java index e021e7d..e94589b 100644 --- a/src/test/java/io/github/gitbucket/markedj/MarkedTest.java +++ b/src/test/java/io/github/gitbucket/markedj/MarkedTest.java @@ -321,6 +321,21 @@ public void testLongTrailingSpacesDoesNotHang() { Marked.marked(sb.toString()); } + // Same reproduction as testLongTrailingSpacesDoesNotHang, but with breaks=true. + // INLINE_BREAKS_RULES built its "text" rule by replacing the bounded "{2,20}" + // quantifier with an unbounded "*", silently re-introducing the O(n^2) blowup + // the bound above was added to fix, only reachable through this option. + @Test(timeout = 2000) + public void testLongTrailingSpacesDoesNotHangWithBreaks() { + StringBuilder sb = new StringBuilder("x"); + for (int i = 0; i < 50_000; i++) { + sb.append(' '); + } + Options options = new Options(); + options.setBreaks(true); + Marked.marked(sb.toString(), options); + } + // Lexer.token() recurses once per nesting level of "> " to handle nested // blockquotes, with no depth limit. A few thousand nesting levels (trivially // reachable via any GitBucket issue/PR/comment/wiki body) blow the JVM call