From e62bb8fb8b4771e036d6aa2d030824482d59a162 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:45:15 -0500 Subject: [PATCH] fix: escape comment content that abruptly closes the comment Escape a leading `>` or `->` in comment data during HTML serialization, so that `#comment('>')` is emitted as `` rather than `-->`. The parser treats `>` and `->` immediately after `-->`, which de-serializes into an empty + * comment followed by a live `` element. Escaping the leading `>` keeps + * the content inside the comment, where it stays inert. + */ +function escapeAbruptClosingCommentTag(rawContent) { + if (rawContent.startsWith('>')) { + return '>' + rawContent.slice(1); + } + if (rawContent.startsWith('->')) { + return '->' + rawContent.slice(2); + } + return rawContent; // fast path +} + /** * Escapes closing comment tag in a comment content. * * For example, given `#comment('-->')`, the content of a comment would be * updated to `-->` to avoid unexpected and unsafe behavior after - * de-serialization. + * de-serialization. Content that abruptly closes an empty comment is + * escaped as well, see `escapeAbruptClosingCommentTag()`. */ function escapeClosingCommentTag(rawContent) { - if (!CLOSING_COMMENT_REGEXP.test(rawContent)) { - return rawContent; // fast path + const content = escapeAbruptClosingCommentTag(rawContent); + if (!CLOSING_COMMENT_REGEXP.test(content)) { + return content; // fast path } - return rawContent.replace(/(--\!?)>/g, '$1>'); + return content.replace(/(--\!?)>/g, '$1>'); } /** diff --git a/test/xss.js b/test/xss.js index aa71a01..066aee9 100644 --- a/test/xss.js +++ b/test/xss.js @@ -572,6 +572,20 @@ exports.verifyEscapeClosingCommentTag = function () { [' ', ' ', 'b', '->a-->b'], + ['--->', '--->'], + + // A `>` that does not start the content does not close the comment. + ['a>', 'a>'], + ['-a>', '-a>'], + [' >', ' >'], + ['a->b', 'a->b'], ]; for (const [rawContent, expected] of cases) { NodeUtils.ɵescapeClosingCommentTag(rawContent).should.equal(expected); @@ -987,3 +1001,79 @@ exports.fallbackRawTextProcessingInstructionEscapesAncestorClosingTag = async fu } }; +exports.commentNodeEscapesAbruptClosingComment = async function () { + // A comment content that starts with `>` or `->` is closed by the parser + // right away (the "abrupt-closing-of-empty-comment" parse error), so the rest + // of the content is parsed as live markup. An inert `Comment` node would + // therefore turn into an XSS vector once an SSR response is parsed by the + // browser, unless the leading `>` is escaped. + const cases = [ + { + data: '>', + expected: '', + }, + { + data: '->', + expected: '', + }, + { + data: '>', + expected: '', + }, + ]; + + for (const testCase of cases) { + const document = domino.createDocument(''); + document.body.appendChild(document.createComment(testCase.data)); + + const serialized = document.body.serialize(); + serialized.should.equal(testCase.expected); + + // The comment stays a single, inert comment node across the round trip. + const reparsedDocument = domino.createDocument('' + serialized + ''); + reparsedDocument.getElementsByTagName('img').length.should.equal(0); + reparsedDocument.getElementsByTagName('script').length.should.equal(0); + reparsedDocument.body.childNodes.length.should.equal(1); + reparsedDocument.body.childNodes[0].nodeType.should.equal(8 /* COMMENT_NODE */); + + const html = document.serialize(); + const alerted = await alertFired(html); + alerted.should.equal(false, 'alert fired for: ' + html); + } +}; + +exports.commentNodePreservesNonClosingAngleBrackets = function () { + // Only a `>` that starts the content closes the comment, everything else + // must be left untouched to keep the serialized comment readable. + const document = domino.createDocument(''); + document.body.appendChild(document.createComment(' a > b -> c ')); + + document.body.serialize().should.equal(''); +}; + +exports.fallbackRawTextCommentNodeEscapesAbruptClosingComment = async function () { + const fallbackTags = ['noscript', 'iframe', 'noembed', 'noframes']; + + for (const tag of fallbackTags) { + // An abrupt comment close combined with an ancestor closing tag has to + // escape both, otherwise the payload breaks out of the comment and out of + // the fallback raw-content element. + const document = domino.createDocument(''); + const el = document.createElement(tag); + el.appendChild(document.createComment(`>`)); + document.body.appendChild(el); + + const serialized = document.body.serialize(); + serialized.should.equal(`<${tag}>`); + + const reparsedDocument = domino.createDocument('' + serialized + ''); + reparsedDocument.getElementsByTagName('img').length.should.equal(0); + + const reparsed = reparsedDocument.body.innerHTML; + const alerted = await alertFired(reparsed); + alerted.should.equal( + false, + `alert fired after normal HTML reparse for abrupt comment in <${tag}>: ` + reparsed, + ); + } +};