leizongmin / js-xss

Sanitize untrusted HTML (to prevent XSS) with a configuration specified by a Whitelist
http://jsxss.com
Other
5.2k stars 629 forks source link

Consider allowCommentTag when parsing tags #167

Closed ronrother closed 4 years ago

ronrother commented 5 years ago

Currently, the allowCommentTag option is not enough to preserve comments when used together with stripIgnoreTag or stripIgnoreTagBody. The following code can be used to reproduce the issue:

<script src="https://rawgit.com/leizongmin/js-xss/master/dist/xss.js"></script>
<script>
var html = filterXSS(
    '<span><!-- a comment --></span>',
    {
        allowCommentTag: true,
        stripIgnoreTagBody: true,
    }
);
alert(html);
</script>

This code, when ran in the browser, yields the value:

<span>[removed]</span>

While using stripIgnoreTag, it outputs:

<span></span>

The proposed fix is to consider the allowCommentTag when deciding whether to remove or preserve a tag.