do you know why CodeQL suggest this? what is wrong in the code?
values = vals.replace(/(^s*<!--)|(-->s*$)|s+/g, '').split(',');
This regular expression only parses –> and not –!> as a HTML comment end tag.
CodeQL
do you know why CodeQL suggest this? what is wrong in the code?
values = vals.replace(/(^s*<!--)|(-->s*$)|s+/g, '').split(',');
This regular expression only parses –> and not –!> as a HTML comment end tag.
CodeQL
2
Answers
Not saying it should look for
--!>
, however if you need an explanation of your regular expression here it is.(^s*<!--)
looks for the lines starting with 0 or more whitespace characters and your<!--
.(-->s*$)
looks for-->
followed by 0 or more white space characters.So you need to add:
(--!>s*$)
to look for the--!>
characters.Though I don’t think I’ve ever seen
--!>
so maybe double check you really do need it. https://www.w3schools.com/tags/tag_comment.aspThis is a good starting point for learning regular expressions: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions
Normally the CodeQL code scanning on GitHub should also include the detailed explanation of the query (maybe this is just cut off on your screenshot). In this case this seems to be Bad HTML filtering regexp. Its description says:
And in the Example section it says:
That corner case for
--!>
is actually described in the HTML specification asincorrectly-closed-comment
error:So summarized, if you choose to use regex for parsing HTML comments then you should for security reasons also consider treating
--!>
as end of an HTML comment as well.