Merge pull request github#9807 from erik-krogh/endFilter

JS: recognize "-->" as a bad tag filter

Author: erik-krogh

javascript/ql/test/query-tests/Security/CWE-116/BadTagFilter/BadTagFilter.expected

@@ -15,3 +15,4 @@
 | tst.js:20:3:20:57 | (<[a-z\\/!$]("[^"]*"\|'[^']*'\|[^'">])*>\|<!(--.*?--\\s*)+>) | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 3 and comments ending with --!> are matched with capture group 1. |
 | tst.js:21:6:21:249 | <(?:(?:!--([\\w\\W]*?)-->)\|(?:!\\[CDATA\\[([\\w\\W]*?)\\]\\]>)\|(?:!DOCTYPE([\\w\\W]*?)>)\|(?:\\?([^\\s\\/<>]+) ?([\\w\\W]*?)[?/]>)\|(?:\\/([A-Za-z][A-Za-z0-9\\-_\\:\\.]*)>)\|(?:([A-Za-z][A-Za-z0-9\\-_\\:\\.]*)((?:\\s+[^"'>]+(?:(?:"[^"]*")\|(?:'[^']*')\|[^>]*))*\|\\/\|\\s+)>)) | This regular expression only parses --> (capture group 1) and not --!> as an HTML comment end tag. |
 | tst.js:22:6:22:33 | <!--([\\w\\W]*?)-->\|<([^>]*?)> | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 1 and comments ending with --!> are matched with capture group 2. |
+| tst.js:31:6:31:8 | --> | This regular expression only parses --> and not --!> as a HTML comment end tag. |

javascript/ql/test/query-tests/Security/CWE-116/BadTagFilter/tst.js

@@ -26,3 +26,10 @@ doFilters(filters)
 
 var strip = '<script([^>]*)>([\\S\\s]*?)<\/script([^>]*)>';  // OK - it's used with the ignorecase flag
 new RegExp(strip, 'gi');
+
+var moreFilters = [
+    /-->/g, // NOT OK - doesn't match --!>
+    /^>|^->|<!--|-->|--!>|<!-$/g, // OK
+];
+
+doFilters(moreFilters);

shared/regex/codeql/regex/nfa/BadTagFilterQuery.qll

@@ -23,9 +23,15 @@ module Make<RegexTreeViewSig TreeImpl> {
     RootTerm root, string str, boolean ignorePrefix, boolean testWithGroups
   ) {
     // the regexp must mention "<" and ">" explicitly.
-    forall(string angleBracket | angleBracket = ["<", ">"] |
-      any(RegExpConstant term | term.getValue().matches("%" + angleBracket + "%")).getRootTerm() =
-        root
+    (
+      forall(string angleBracket | angleBracket = ["<", ">"] |
+        any(RegExpConstant term | term.getValue().matches("%" + angleBracket + "%")).getRootTerm() =
+          root
+      )
+      or
+      // or contain "-->" / "--!>" / "<--" / "<!--"
+      root =
+        any(RegExpConstant term | term.getValue() = ["-->", "--!>", "<--", "<!--"]).getRootTerm()
     ) and
     ignorePrefix = true and
     (
@@ -40,7 +46,7 @@ module Make<RegexTreeViewSig TreeImpl> {
           "<script src='foo'></script>", "<SCRIPT>foo</SCRIPT>", "<script\tsrc=\"foo\"/>",
           "<script\tsrc='foo'></script>", "<sCrIpT>foo</ScRiPt>",
           "<script src=\"foo\">foo</script >", "<script src=\"foo\">foo</script foo=\"bar\">",
-          "<script src=\"foo\">foo</script\t\n bar>"
+          "<script src=\"foo\">foo</script\t\n bar>", "-->", "--!>", "--"
         ] and
       testWithGroups = false
     )
@@ -107,6 +113,12 @@ module Make<RegexTreeViewSig TreeImpl> {
           ") and not --!> as an HTML comment end tag."
     )
     or
+    // CVE-2021-4231 - matching only "-->" but not "--!>".
+    regexp.matches("-->") and
+    not regexp.matches("--!>") and
+    not regexp.matches("--") and
+    msg = "This regular expression only parses --> and not --!> as a HTML comment end tag."
+    or
     regexp.matches("<!-- foo -->") and
     not regexp.matches("<!-- foo\n -->") and
     not regexp.matches("<!- foo ->") and