Merge pull request #6015 from erik-krogh/resolve

Approved by asgerf

Author: codeql-ci

javascript/change-notes/2021-06-04-resolve.md

@@ -0,0 +1,4 @@
+lgtm,codescanning
+* Paths used with the [resolve](https://npmjs.com/package/resolve) command are seen as sinks for the `js/path-injection` query.
+  Affected packages are
+    [resolve](https://npmjs.com/package/resolve)

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

@@ -577,6 +577,17 @@ module TaintedPath {
     }
   }
 
+  /**
+   * An expression whose value is resolved to a module using the [resolve](http://npmjs.com/package/resolve) library.
+   */
+  class ResolveModuleSink extends Sink {
+    ResolveModuleSink() {
+      this = API::moduleImport("resolve").getACall().getArgument(0)
+      or
+      this = API::moduleImport("resolve").getMember("sync").getACall().getArgument(0)
+    }
+  }
+
   /**
    * A path argument to a file system access.
    */

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

@@ -2594,6 +2594,18 @@ nodes
 | 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") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") |
 | 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") |
@@ -7090,6 +7102,8 @@ edges
 | tainted-access-paths.js:31:23:31:25 | obj | tainted-access-paths.js:31:23:31:30 | obj.sub4 |
 | tainted-access-paths.js:31:23:31:25 | obj | tainted-access-paths.js:31:23:31:30 | obj.sub4 |
 | tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") |
+| tainted-require.js:12:29:12:47 | req.param("module") | tainted-require.js:12:29:12:47 | req.param("module") |
+| tainted-require.js:14:11:14:29 | req.param("module") | tainted-require.js:14:11:14:29 | req.param("module") |
 | tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
 | tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") |
 | tainted-sendFile.js:18:43:18:58 | req.param("dir") | tainted-sendFile.js:18:43:18:58 | req.param("dir") |
@@ -8304,6 +8318,8 @@ edges
 | tainted-access-paths.js:30:23:30:30 | obj.sub4 | tainted-access-paths.js:6:24:6:30 | req.url | tainted-access-paths.js:30:23:30:30 | obj.sub4 | This path depends on $@. | tainted-access-paths.js:6:24:6:30 | req.url | a user-provided value |
 | tainted-access-paths.js:31:23:31:30 | obj.sub4 | tainted-access-paths.js:6:24:6:30 | req.url | tainted-access-paths.js:31:23:31:30 | obj.sub4 | This path depends on $@. | tainted-access-paths.js:6:24:6:30 | req.url | 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-require.js:12:29:12:47 | req.param("module") | tainted-require.js:12:29:12:47 | req.param("module") | tainted-require.js:12:29:12:47 | req.param("module") | This path depends on $@. | tainted-require.js:12:29:12:47 | req.param("module") | a user-provided value |
+| tainted-require.js:14:11:14:29 | req.param("module") | tainted-require.js:14:11:14:29 | req.param("module") | tainted-require.js:14:11:14:29 | req.param("module") | This path depends on $@. | tainted-require.js:14:11:14:29 | 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 |
 | tainted-sendFile.js:18:43:18:58 | req.param("dir") | tainted-sendFile.js:18:43:18:58 | req.param("dir") | tainted-sendFile.js:18:43:18:58 | req.param("dir") | This path depends on $@. | tainted-sendFile.js:18:43:18:58 | req.param("dir") | a user-provided value |

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

@@ -6,3 +6,12 @@ app.get('/some/path', function(req, res) {
   // BAD: loading a module based on un-sanitized query parameters
   var m = require(req.param("module"));
 });
+
+const resolve = require("resolve");
+app.get('/some/path', function(req, res) {
+  var module = resolve.sync(req.param("module")); // NOT OK - resolving module based on query parameters
+
+  resolve(req.param("module"), { basedir: __dirname }, function(err, res) { // NOT OK - resolving module based on query parameters
+    var module = res;
+  });
+});