add support for "../" prefixes in sanitizer

Author: erik-krogh

javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

@@ -368,10 +368,13 @@ module TaintedPath {
       this = startsWith and
       relativeCall = DataFlow::moduleImport("path").getAMemberCall("relative") and
       startsWith.getBaseString().getALocalSource() = relativeCall and
-      exists(DataFlow::Node subString | subString = startsWith.getSubstring() |
-        subString.mayHaveStringValue("..")
+      exists(DataFlow::Node subString, string prefix |
+        subString = startsWith.getSubstring() and
+        (prefix = ".." or prefix = "../")
+      |
+        subString.mayHaveStringValue(prefix)
         or
-        subString.(StringOps::ConcatenationRoot).getFirstLeaf().mayHaveStringValue("..")
+        subString.(StringOps::ConcatenationRoot).getFirstLeaf().mayHaveStringValue(prefix)
       )
     }
 

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

@@ -1287,6 +1287,11 @@ nodes
 | normalizedPaths.js:270:21:270:24 | path |
 | normalizedPaths.js:270:21:270:24 | path |
 | normalizedPaths.js:270:21:270:24 | path |
+| normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:278:21:278:24 | path |
 | tainted-require.js:7:19:7:37 | req.param("module") |
 | tainted-require.js:7:19:7:37 | req.param("module") |
 | tainted-require.js:7:19:7:37 | req.param("module") |
@@ -3682,6 +3687,14 @@ edges
 | normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:270:21:270:24 | path |
 | normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:270:21:270:24 | path |
 | normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:270:21:270:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
+| normalizedPaths.js:254:7:254:47 | path | normalizedPaths.js:278:21:278:24 | path |
 | normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
 | normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
 | normalizedPaths.js:254:14:254:47 | pathMod ... y.path) | normalizedPaths.js:254:7:254:47 | path |
@@ -4478,6 +4491,7 @@ edges
 | normalizedPaths.js:256:19:256:22 | path | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:256:19:256:22 | path | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
 | normalizedPaths.js:262:21:262:24 | path | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:262:21:262:24 | path | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
 | normalizedPaths.js:270:21:270:24 | path | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:270:21:270:24 | path | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
+| normalizedPaths.js:278:21:278:24 | path | normalizedPaths.js:254:33:254:46 | req.query.path | normalizedPaths.js:278:21:278:24 | path | This path depends on $@. | normalizedPaths.js:254:33:254:46 | req.query.path | a user-provided value |
 | tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") | This path depends on $@. | tainted-require.js:7:19:7:37 | req.param("module") | a user-provided value |
 | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | This path depends on $@. | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | a user-provided value |
 | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | This path depends on $@. | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | a user-provided value |

javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js

@@ -269,6 +269,14 @@ app.get('/relative-startswith', (req, res) => {
   if (relativePath.indexOf('..' + pathModule.sep) === 0) {
     fs.readFileSync(path); // NOT OK!
   } else {
-	  fs.readFileSync(newpath); // OK! 
+    fs.readFileSync(newpath); // OK!
+  }
+
+  let newpath = pathModule.normalize(p);
+  var relativePath = path.relative(path.normalize(workspaceDir), newpath);
+  if (relativePath.indexOf('../') === 0) {
+    fs.readFileSync(path); // NOT OK!
+  } else {
+    fs.readFileSync(newpath); // OK! 
   }
 });