28

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.

Improve this question
3
  • 5
    Wouldn't it be better to report that via mail?
    – Octavian Helm
    Jun 15, 2011 at 14:14
  • Yep, definately causes a problem.
    – Engineer2021
    Jun 15, 2011 at 14:15
  • 2
    Too late, mwah ha haaa! ...kidding ;)
    – Michael
    Jun 15, 2011 at 14:44

1 Answer 1

Reset to default
25

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).

Improve this answer
10
  • What's wrong with <!--.*?--\s*> for the HTML 4 version?
    – Justin Morgan
    Jun 16, 2011 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 StaffMod
    Jun 16, 2011 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, 2011 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 StaffMod
    Jun 16, 2011 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, 2011 at 14:20
  • @Tomalak: Thanks. Yeah, I guess that'd work as well for the 99 percent case.
    – balpha StaffMod
    Jun 16, 2011 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, 2011 at 14:36
  • @JustinMorgan: refiddle.com/19a is what I meant (nice site by the way; didn't know it)
    – balpha StaffMod
    Jun 16, 2011 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, 2011 at 2:15
  • @balpha I've just found your answer since it answers my not-to-be-asked-now question: I wanted to put an outdated part of my answer on SO into an HTML comment, but markdown broke due to >> being part of the commented text (coming from copied interactive MATLAB output). Your answer explained the problem: the tag sanitizer ate the post. Since this is an old post of yours: is there any chance the >> characters could be protected from the sanitizer? Or is there any reasonable work-around to comment larger blocks of answers with tag-y contents? (BTW your "tag sanitizer" link seems rotten now.)
    – Andras Deak -- Слава Україні
    Jan 17, 2016 at 14:32

You must log in to answer this question.

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