skip to Main Content

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

enter image description here

2

Answers


  1. Not saying it should look for --!>, however if you need an explanation of your regular expression here it is.

    vals.replace(/(^s*<!--)|(-->s*$)|s+/g, '').split(',');
    

    (^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.

    vals.replace(/(^s*<!--)|(-->s*$)|(--!>s*$)|s+/g, '').split(',');
    

    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.asp

    This is a good starting point for learning regular expressions: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions

    Login or Signup to reply.
  2. 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:

    It is possible to match some single HTML tags using regular expressions (parsing general HTML using regular expressions is impossible). However, if the regular expression is not written well it might be possible to circumvent it, which can lead to cross-site scripting or other security issues.

    Some of these mistakes are caused by browsers having very forgiving HTML parsers, and will often render invalid HTML containing syntax errors. Regular expressions that attempt to match HTML should also recognize tags containing such syntax errors.

    And in the Example section it says:

    Other corner cases include that HTML comments can end with --!> […]

    That corner case for --!> is actually described in the HTML specification as incorrectly-closed-comment error:

    This error occurs if the parser encounters a comment that is closed by the "--!>" code point sequence. The parser treats such comments as if they are correctly closed by the "-->" code point sequence.

    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.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search