What is meta? ×
Meta Stack Exchange is where users like you discuss bugs, features, and support issues that affect the software powering all 133 Stack Exchange communities.

Possible DoS with html comments in questions and answers

<!--------------------------------------------------------------------------------------------------------------------------------------------------

You can try copy pasting above in question and answer boxes.

Tested on Chrome and Opera, It freeze or browser ask me to kill the page.

99% CPU usage on browser and server request like this

enter image description here

As you see post request took 1.3 min on server side too.

Edit: Firefox seems to be okay on client side.

share|improve this question
5  
Wouldn't it be better to report that via mail? –  Octavian Damiean Jun 15 '11 at 14:14
    
Yep, definately causes a problem. –  staticx Jun 15 '11 at 14:15
2  
Too late, mwah ha haaa! ...kidding ;) –  Mikaveli Jun 15 '11 at 14:44

1 Answer 1

up vote 22 down vote accepted

The original Markdown implementation (and probably all its ports, including Showdown and MarkdownSharp) basically uses this regular expression to find HTML comments:

<!(--.*?--\s*)+>

This boils down to

(.*)*

which is bound for catastrophe. Granted, the HTML4 definition of comments isn't very helpful when it comes to specifying what precisely constitues a comment:

HTML comments have the following syntax:

<!-- this is a comment -->
<!-- and so is this one,
    which occupies more than one line -->

White space is not permitted between the markup declaration open delimiter("<!") and the comment open delimiter ("--"), but is permitted between the comment close delimiter ("--") and the markup declaration close delimiter (">"). A common error is to include a string of hyphens ("---") within a comment. Authors should avoid putting two or more adjacent hyphens inside comments.

– that's like defining a carnivore using a picture of a cat, with a footnote stating that there may be versions with more than four legs.

The HTML5 definition is much better:

Comments consist of the following parts, in exactly the following order:

  1. the comment start delimiter "<!--"
  2. text
  3. the comment end delimiter "-->"

The text part of comments has the following restrictions:

  • must not start with a ">" character
  • must not start with the string "->"
  • must not contain the string "--"
  • must not end with a "-" character

– and also lends itself to a much safer regular expression:

<!(--(?:|(?:[^>-]|-[^>])(?:[^-]|-[^-])*)--)>

(note this is only 99% correct, because it considers <!------> a valid comment, which it isn't – but that's the only deviance, and not really a real-life problem).

So from now on, that's what we'll be using. It should be mentioned that while a > is a valid character in a comment from both the W3C specification and the above Regex, it's not going to work correctly in our Markdown, because the tag sanitizer will eat it:

<!-- love > hate -->

turns into

hate -->

Anyone running Markdown on their site should take a look at their implementation; while they don't have to use the above change, replacing the original Regex with something safer might be in order.

The above change has been folded into our Google code repositories of both Showdown (the JavaScript version) and MarkdownSharp (the C# version).

share|improve this answer
    
What's wrong with <!--.*?--\s*> for the HTML 4 version? –  Justin Morgan Jun 16 '11 at 14:04
    
@JustinMorgan: The problem is the + at the end. See the article on catastrophic backtracking I linked to; it's a pretty good writeup. –  balpha Jun 16 '11 at 14:08
    
I mean without the +. Just <!--.*?--\s*>, with no nested quantifiers. I don't really see why the + was in there anyway; it seems unnecessary. There must be some kind of edge case I'm not figuring out here, because I tested it with whatever weirdness I could think of. –  Justin Morgan Jun 16 '11 at 14:12
    
@JustinMorgan: Ask John Gruber :) I mainly went with the HTML5 version because it has a clear definition and not so many edge cases. But your version also has the problem that the part of Markdown that handles multiline comments also needs to match stuff coming after the comment (namely newlines), so imagine putting \n after your version, and having a comment without trailing newline at the top, and a comment (with trailing newline) somewhere below. Everything between those two will be handled incorrectly (that's what caused meta.stackexchange.com/questions/74754). –  balpha Jun 16 '11 at 14:19
    
You have a typo in your regex. Other than that - how would you assess <!--(?:(?!-->)[\s\S])*--> as an alternative? –  Tomalak Jun 16 '11 at 14:20
    
@Tomalak: Thanks. Yeah, I guess that'd work as well for the 99 percent case. –  balpha Jun 16 '11 at 14:27
    
@balpha - For some reason, I was thinking of comments (what I'm typing now) instead of all HTML comments in questions and answers. Derp. Anyway, <!--[\s\S]*?--\s*> handles newlines, and seems to do fine at it. It's possible I misunderstood the edge case you described. I'm overthinking this anyway, I suppose. –  Justin Morgan Jun 16 '11 at 14:36
    
@JustinMorgan: refiddle.com/19a is what I meant (nice site by the way; didn't know it) –  balpha Jun 16 '11 at 14:50
1  
This part from the HTML 4 spec is not the definition of a comment - it is in a section named On SGML and HTML, and is a informal description of the stuff which you can read formally in the SGML specification. In theory (in SGML), a comment is a section starting with -- and ending with --, but only inside a <!...> section. Thus <!-- comment -- -- another comment -- > would be a valid section consisting of two comments. (The same principle can be seen in the DTD syntax). But in practice most browsers do not have SGML parsers, thus the recommendation. –  Paŭlo Ebermann Jun 17 '11 at 2:15

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .