JS: incomplete sanitization now also works with RegExp objects

github · Nov 27, 2024 · c003a81 · c003a81
1 parent f179747
commit c003a81
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql b/javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql
@@ -23,7 +23,7 @@ string metachar() { result = "'\"\\&<>\n\r\t*|{}[]%$".charAt(_) }
 
 /** Gets a string matched by `e` in a `replace` call. */
 string getAMatchedString(DataFlow::Node e) {
-  result = e.(DataFlow::RegExpLiteralNode).getRoot().getAMatchedString()
+  result = e.(DataFlow::RegExpCreationNode).getRoot().getAMatchedString()
   or
   result = e.getStringValue()
 }
@@ -52,7 +52,7 @@ predicate isSimpleAlt(RegExpAlt t) { forall(RegExpTerm ch | ch = t.getAChild() |
  * Holds if `mce` is of the form `x.replace(re, new)`, where `re` is a global
  * regular expression and `new` prefixes the matched string with a backslash.
  */
-predicate isBackslashEscape(StringReplaceCall mce, DataFlow::RegExpLiteralNode re) {
+predicate isBackslashEscape(StringReplaceCall mce, DataFlow::RegExpCreationNode re) {
   mce.isGlobal() and
   re = mce.getRegExp() and
   (
@@ -72,7 +72,7 @@ predicate allBackslashesEscaped(DataFlow::Node nd) {
   nd instanceof JsonStringifyCall
   or
   // check whether `nd` itself escapes backslashes
-  exists(DataFlow::RegExpLiteralNode rel | isBackslashEscape(nd, rel) |
+  exists(DataFlow::RegExpCreationNode rel | isBackslashEscape(nd, rel) |
     // if it's a complex regexp, we conservatively assume that it probably escapes backslashes
     not isSimple(rel.getRoot()) or
     getAMatchedString(rel) = "\\"

diff --git a/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/tst.js b/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/tst.js
@@ -338,19 +338,19 @@ function typicalBadHtmlSanitizers(s) {
 }
 
 function bad18NewRegExp(p) {
-	return p.replace(new RegExp("\\.\\./"), ""); // NOT OK -- should be flagged, but currently checking only for literals
+	return p.replace(new RegExp("\\.\\./"), ""); // NOT OK
 }
 
 function bad4NewRegExpG(s) {
-	return s.replace(new RegExp("\'","g"), "\\$&"); // NOT OK -- should be flagged, but currently checking only for literals
+	return s.replace(new RegExp("\'","g"), "\\$&"); // NOT OK
 }
 
 function bad4NewRegExp(s) {
-	return s.replace(new RegExp("\'"), "\\$&"); // NOT OK -- should be flagged, but currently checking only for literals
+	return s.replace(new RegExp("\'"), "\\$&"); // NOT OK
 }
 
 function bad4NewRegExpUnknown(s) {
-	return s.replace(new RegExp("\'", unknownFlags()), "\\$&"); // NOT OK -- should be flagged, but currently checking only for literals
+	return s.replace(new RegExp("\'", unknownFlags()), "\\$&"); // NOT OK
 }
 
 function newlinesNewReGexp(s) {
@@ -359,9 +359,9 @@ function newlinesNewReGexp(s) {
 	x.replace(new RegExp("\n", "g"), "").replace(x, y); // OK
 	x.replace(x, y).replace(new RegExp("\n", "g"), ""); // OK
 
-	x.replace(new RegExp("\n"), "").replace(x, y); // NOT OK -- should be flagged, but currently checking only for literals
-	x.replace(x, y).replace(new RegExp("\n"), ""); // NOT OK -- should be flagged, but currently checking only for literals
+	x.replace(new RegExp("\n"), "").replace(x, y); // NOT OK
+	x.replace(x, y).replace(new RegExp("\n"), ""); // NOT OK
 
-	x.replace(new RegExp("\n", unknownFlags()), "").replace(x, y); // OK
-	x.replace(x, y).replace(new RegExp("\n", unknownFlags()), ""); // OK
+	x.replace(new RegExp("\n", unknownFlags()), "").replace(x, y); // OK -- Should not be flagged but now it is 
+	x.replace(x, y).replace(new RegExp("\n", unknownFlags()), ""); // OK -- Should not be flagged but now it is 
 }