[comments re] halt with CPU 100% on some html

[puzrin]

Put this into dingus:

foo <!--- This is what the user will see during the configuration ->

Bug in tag detection regexp. Found by @fengmk2

Upd Verify that CDATA regexp does not have the same vulnerability

Upd2 The same problem in CDATA #267

[puzrin]

Problem is at https://github.com/jgm/CommonMark/blob/master/js/lib/inlines.js#L36

var HTMLCOMMENT = "<!--([^-]+|[-][^-]+)*-->";

What are those complex conditions for? May be use one below?

var HTMLCOMMENT = "<!--[\s\S]*?-->";

It works.

[mildsunrise]

From the spec:

An HTML comment consists of the string <!--, a string of characters not including the string --, and the string -->.

I think the ([^-]+|[-][^-]+)* aims to make sure comments don't contain --.

[puzrin]

Ah, got it. Well:

1. It does not match all conditions anyway http://www.w3.org/TR/html-markup/syntax.html#comments
2. Browsers don't care additional conditions (no --, no >, no ending -)

I think it's more simple & safe to remove those from regexp. If anyone wish to get better safety, he will pas result via sanitizer. Or we could force escaping those. But those should not be considered as "not comment" block.

[puzrin]

Upd: /<!--[\s\S]*?-->/ can fail on <!-->...<!--> & <!--->... <!-->, but we can do additional checks later, instead of unsafe regexp magic.

[mildsunrise]

@puzrin I reported this just after posting my comment, see #264. They seem to have taken that from XML which is more permissive.

[puzrin]

Memo. Verify that CDATA regexp does not have the same vulnerability, because it's built with the same principle.

Updated the first message.

[jgm]

These regexes work fine with cmark (and re2c, which creates a non-backtracking DFA from the regex). I guess the regex libraries in javascript allow pathological backtracking. Presumably there are ways to write equivalent regexes that won't be pathological? (In the case of the comment regex, it's probably worth matching HTML5 behavior, #264.)

[jgm]

How about this for a non-pathological regex that matches HTML5 behavior?

/<!--(?!<)(?!->)(?:-?[^-])+-->/

[puzrin]

I've fixed comments in markdown-it with more simple regexp & postponed check:

markdown-it/markdown-it@792f386

Don't know if it's good for ref implementation.

[fengmk2]

@puzrin can you publish a new version markdown-it to npm?

[puzrin]

@fengmk2 please, wait ~ 1 day max. We are about to finish 3.0.0 milestone.

Feel free to use markdown-it tracker for related questions.