Merge pull request #2730 from esbena/js/model-path-parse

Approved by asgerf

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

@@ -1045,4 +1045,25 @@ module NodeJSLib {
       i = 0 and result = this.getArgument(i)
     }
   }
+
+  /**
+   * Provides predicates for working with the "path" module and its platform-specific instances as a single module.
+   */
+  module Path {
+    /**
+     * Gets a node that imports the "path" module, or one of its platform-specific instances.
+     */
+    DataFlow::SourceNode moduleImport() {
+      result = DataFlow::moduleImport("path") or
+      result = DataFlow::moduleMember("path", "posix") or
+      result = DataFlow::moduleMember("path", "win32")
+    }
+
+    /**
+     * Gets an access to member `member` of the "path" module, or one of its platform-specific instances.
+     */
+    DataFlow::SourceNode moduleMember(string member) {
+      result = moduleImport().getAPropertyRead(member)
+    }
+  }
 }

javascript/ql/src/semmle/javascript/frameworks/UriLibraries.qll

@@ -401,4 +401,33 @@ private module ClosureLibraryUri {
       succ = uri
     }
   }
+
+  /**
+   * Provides classes for working with [path](https://nodejs.org/api/path.html) code.
+   */
+  module path {
+    /**
+     * A taint step in the path module.
+     */
+    private class Step extends UriLibraryStep, DataFlow::CallNode {
+      DataFlow::Node src;
+
+      Step() {
+        exists(DataFlow::SourceNode ref |
+          ref = NodeJSLib::Path::moduleMember("parse") or
+          // a ponyfill: https://www.npmjs.com/package/path-parse
+          ref = DataFlow::moduleImport("path-parse") or
+          ref = DataFlow::moduleMember("path-parse", "posix") or
+          ref = DataFlow::moduleMember("path-parse", "win32")
+        |
+          this = ref.getACall() and
+          src = getAnArgument()
+        )
+      }
+
+      override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+        pred = src and succ = this
+      }
+    }
+  }
 }

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

@@ -142,7 +142,7 @@ module TaintedPath {
       or
       // path.join()
       exists(DataFlow::CallNode join, int n |
-        join = DataFlow::moduleMember("path", "join").getACall()
+        join = NodeJSLib::Path::moduleMember("join").getACall()
       |
         src = join.getArgument(n) and
         dst = join and

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

@@ -124,7 +124,7 @@ module TaintedPath {
     DataFlow::Node output;
 
     NormalizingPathCall() {
-      this = DataFlow::moduleMember("path", "normalize").getACall() and
+      this = NodeJSLib::Path::moduleMember("normalize").getACall() and
       input = getArgument(0) and
       output = this
     }
@@ -148,7 +148,7 @@ module TaintedPath {
     DataFlow::Node output;
 
     ResolvingPathCall() {
-      this = DataFlow::moduleMember("path", "resolve").getACall() and
+      this = NodeJSLib::Path::moduleMember("resolve").getACall() and
       input = getAnArgument() and
       output = this
       or
@@ -180,7 +180,7 @@ module TaintedPath {
     DataFlow::Node output;
 
     NormalizingRelativePathCall() {
-      this = DataFlow::moduleMember("path", "relative").getACall() and
+      this = NodeJSLib::Path::moduleMember("relative").getACall() and
       input = getAnArgument() and
       output = this
     }
@@ -205,7 +205,7 @@ module TaintedPath {
 
     PreservingPathCall() {
       exists(string name | name = "dirname" or name = "toNamespacedPath" |
-        this = DataFlow::moduleMember("path", name).getACall() and
+        this = NodeJSLib::Path::moduleMember(name).getACall() and
         input = getAnArgument() and
         output = this
       )
@@ -244,7 +244,7 @@ module TaintedPath {
     // ".." + path.sep
     exists(StringOps::Concatenation conc | node = conc |
       conc.getOperand(0).getStringValue() = ".." and
-      conc.getOperand(1).getALocalSource() = DataFlow::moduleMember("path", "sep") and
+      conc.getOperand(1).getALocalSource() = NodeJSLib::Path::moduleMember("sep") and
       conc.getNumOperand() = 2
     )
   }
@@ -311,7 +311,7 @@ module TaintedPath {
 
     IsAbsoluteSanitizer() {
       exists(DataFlow::CallNode call | this = call |
-        call = DataFlow::moduleMember("path", "isAbsolute").getACall() and
+        call = NodeJSLib::Path::moduleMember("isAbsolute").getACall() and
         operand = call.getArgument(0) and
         polarity = true and
         negatable = true

javascript/ql/test/library-tests/frameworks/UriLibraries/UriLibraryStep.expected

@@ -24,6 +24,12 @@
 | closureUri.js:23:1:23:18 | utils.getPath(uri) | closureUri.js:23:15:23:17 | uri | closureUri.js:23:1:23:18 | utils.getPath(uri) |
 | closureUri.js:27:1:27:23 | stringU ... code(x) | closureUri.js:27:22:27:22 | x | closureUri.js:27:1:27:23 | stringU ... code(x) |
 | closureUri.js:28:1:28:23 | stringU ... code(x) | closureUri.js:28:22:28:22 | x | closureUri.js:28:1:28:23 | stringU ... code(x) |
+| path-parse.js:4:1:4:13 | path.parse(x) | path-parse.js:4:12:4:12 | x | path-parse.js:4:1:4:13 | path.parse(x) |
+| path-parse.js:5:1:5:13 | path_parse(x) | path-parse.js:5:12:5:12 | x | path-parse.js:5:1:5:13 | path_parse(x) |
+| path-parse.js:6:1:6:19 | path.posix.parse(x) | path-parse.js:6:18:6:18 | x | path-parse.js:6:1:6:19 | path.posix.parse(x) |
+| path-parse.js:7:1:7:19 | path_parse.posix(x) | path-parse.js:7:18:7:18 | x | path-parse.js:7:1:7:19 | path_parse.posix(x) |
+| path-parse.js:8:1:8:19 | path.win32.parse(x) | path-parse.js:8:18:8:18 | x | path-parse.js:8:1:8:19 | path.win32.parse(x) |
+| path-parse.js:9:1:9:19 | path_parse.win32(x) | path-parse.js:9:18:9:18 | x | path-parse.js:9:1:9:19 | path_parse.win32(x) |
 | punycode.js:3:9:3:26 | punycode.decode(x) | punycode.js:3:25:3:25 | x | punycode.js:3:9:3:26 | punycode.decode(x) |
 | punycode.js:5:5:5:22 | punycode.encode(x) | punycode.js:5:21:5:21 | x | punycode.js:5:5:5:22 | punycode.encode(x) |
 | punycode.js:7:5:7:25 | punycod ... code(x) | punycode.js:7:24:7:24 | x | punycode.js:7:5:7:25 | punycod ... code(x) |

javascript/ql/test/library-tests/frameworks/UriLibraries/path-parse.js

@@ -0,0 +1,9 @@
+const path = require('path');
+const path_parse = require('path-parse');
+
+path.parse(x);
+path_parse(x);
+path.posix.parse(x);
+path_parse.posix(x);
+path.win32.parse(x);
+path_parse.win32(x);