Fix breaking out of comments

Author: wooorm

Diff for: lib/comment.js

@@ -1,7 +1,18 @@
 'use strict'
 
+var xtend = require('xtend')
+var entities = require('stringify-entities')
+
 module.exports = serializeComment
 
+// See: <https://html.spec.whatwg.org/multipage/syntax.html#comments>
+var breakout = /^>|^->|<!--|-->|--!>|<!-$/g
+var subset = ['<', '>']
+
 function serializeComment(ctx, node) {
-  return '<!--' + node.value + '-->'
+  return '<!--' + node.value.replace(breakout, encode) + '-->'
+
+  function encode($0) {
+    return entities($0, xtend(ctx.entities, {subset: subset}))
+  }
 }

Diff for: test/comment.js

@@ -14,15 +14,38 @@ test('`comment`', function(t) {
   t.deepEqual(
     to(u('comment', 'AT&T')),
     '<!--AT&T-->',
-    'should not encode `comment`s (#1)'
+    'should not encode `comment`s'
   )
 
-  // No way to get around this.
-  t.deepEqual(
-    to(u('comment', '-->')),
-    '<!---->-->',
-    'should not encode `comment`s (#2)'
-  )
+  // https://html.spec.whatwg.org/multipage/syntax.html#comments
+  // Optionally, text, with the additional restriction that the text must not
+  // start with the string `>`, nor start with the string `->`, nor contain the
+  // strings `<!--`, `-->`, or `--!>`, nor end with the string `<!-`.
+  var matrix = [
+    ['>a', '&#x3E;a'],
+    ['->a', '-&#x3E;a'],
+    ['a<!--b', 'a&#x3C;!--b'],
+    ['a-->b', 'a--&#x3E;b'],
+    ['a--!>b', 'a--!&#x3E;b'],
+    ['a<!-', 'a&#x3C;!-'],
+    // Not at start:
+    ['a>'],
+    ['a->'],
+    // Not at end:
+    ['a<!-b']
+  ]
+
+  matrix.forEach(function(d) {
+    var input = d[0]
+    var output = d[1] || d[0]
+    var ok = d[1] === undefined
+
+    t.deepEqual(
+      to(u('comment', input)),
+      '<!--' + output + '-->',
+      'security: should ' + (ok ? 'allow' : 'prevent') + ' `' + input + '`'
+    )
+  })
 
   t.end()
 })

Diff for: test/security.js

@@ -8,8 +8,8 @@ var to = require('..')
 test('security', function(t) {
   t.equal(
     to(u('root', [u('comment', '--><script>alert(1)</script><!--')])),
-    '<!----><script>alert(1)</script><!---->',
-    'comments can break out of their context (unsafe)'
+    '<!----&#x3E;<script>alert(1)</script>&#x3C;!---->',
+    'comments cannot break out of their context (safe)'
   )
 
   t.equal(