Merge pull request #1876 from esben-semmle/js/more-delimiter-stripping-whitelisting

Approved by xiemaisi

change-notes/1.23/analysis-javascript.md

@@ -19,6 +19,7 @@
 
 | **Query**                      | **Expected impact**          | **Change**                                                                |
 |--------------------------------|------------------------------|---------------------------------------------------------------------------|
+| Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false-positive results | This rule now recognizes additional ways delimiters can be stripped away. |
 | Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |

javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql

@@ -122,6 +122,10 @@ predicate isDelimiterUnwrapper(
     left = "{" and right = "}"
     or
     left = "(" and right = ")"
+    or
+    left = "\"" and right = "\""
+    or
+    left = "'" and right = "'"
   |
     removesFirstOccurence(leftUnwrap, left) and
     removesFirstOccurence(rightUnwrap, right) and

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

@@ -192,3 +192,8 @@ app.get('/some/path', function(req, res) {
 	var indirect = /'/;
 	return s.replace(indirect, ""); // NOT OK
 });
+
+(function (s) {
+	s.replace('"', '').replace('"', ''); // OK
+	s.replace("'", "").replace("'", ""); // OK
+});