Merge pull request #2387 from max-schaefer/js/incomplete-dotdot-sanitization

Approved by asger-semmle

Author: semmle-qlci

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

@@ -166,6 +166,9 @@ where
         // URL encoder
         repl.getArgument(1).getStringValue().regexpMatch(urlEscapePattern)
       )
+      or
+      // path sanitizer
+      (m = ".." or m = "/.." or m = "../" or m = "/../")
     ) and
     // don't flag replace operations in a loop
     not DataFlow::valueNode(repl.getReceiver()) = DataFlow::valueNode(repl).getASuccessor+() and

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

@@ -28,3 +28,4 @@
 | tst.js:148:2:148:10 | x.replace | This replaces only the first occurrence of "\\n". |
 | tst.js:149:2:149:24 | x.repla ... replace | This replaces only the first occurrence of "\\n". |
 | tst.js:193:9:193:17 | s.replace | This replaces only the first occurrence of /'/. |
+| tst.js:202:10:202:18 | p.replace | This replaces only the first occurrence of "/../". |

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

@@ -197,3 +197,7 @@ app.get('/some/path', function(req, res) {
 	s.replace('"', '').replace('"', ''); // OK
 	s.replace("'", "").replace("'", ""); // OK
 });
+
+function bad18(p) {
+  return p.replace("/../", ""); // NOT OK
+}