diff --git a/src/main/java/io/github/gitbucket/markedj/Grammer.java b/src/main/java/io/github/gitbucket/markedj/Grammer.java
index a9945d1..c8a2740 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..e94589b 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,50 @@ 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());
+ }
+
+ // 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
+ // 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());
+ }
}