Skip to content

Commit fec3985

Browse files
authored
Merge pull request github#6015 from erik-krogh/resolve
Approved by asgerf
2 parents 6279c67 + 5961dd1 commit fec3985

File tree

4 files changed

+40
-0
lines changed

4 files changed

+40
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* Paths used with the [resolve](https://npmjs.com/package/resolve) command are seen as sinks for the `js/path-injection` query.
3+
Affected packages are
4+
[resolve](https://npmjs.com/package/resolve)

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,17 @@ module TaintedPath {
577577
}
578578
}
579579

580+
/**
581+
* An expression whose value is resolved to a module using the [resolve](http://npmjs.com/package/resolve) library.
582+
*/
583+
class ResolveModuleSink extends Sink {
584+
ResolveModuleSink() {
585+
this = API::moduleImport("resolve").getACall().getArgument(0)
586+
or
587+
this = API::moduleImport("resolve").getMember("sync").getACall().getArgument(0)
588+
}
589+
}
590+
580591
/**
581592
* A path argument to a file system access.
582593
*/

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2594,6 +2594,18 @@ nodes
25942594
| tainted-require.js:7:19:7:37 | req.param("module") |
25952595
| tainted-require.js:7:19:7:37 | req.param("module") |
25962596
| tainted-require.js:7:19:7:37 | req.param("module") |
2597+
| tainted-require.js:12:29:12:47 | req.param("module") |
2598+
| tainted-require.js:12:29:12:47 | req.param("module") |
2599+
| tainted-require.js:12:29:12:47 | req.param("module") |
2600+
| tainted-require.js:12:29:12:47 | req.param("module") |
2601+
| tainted-require.js:12:29:12:47 | req.param("module") |
2602+
| tainted-require.js:12:29:12:47 | req.param("module") |
2603+
| tainted-require.js:14:11:14:29 | req.param("module") |
2604+
| tainted-require.js:14:11:14:29 | req.param("module") |
2605+
| tainted-require.js:14:11:14:29 | req.param("module") |
2606+
| tainted-require.js:14:11:14:29 | req.param("module") |
2607+
| tainted-require.js:14:11:14:29 | req.param("module") |
2608+
| tainted-require.js:14:11:14:29 | req.param("module") |
25972609
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
25982610
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
25992611
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
@@ -7090,6 +7102,8 @@ edges
70907102
| tainted-access-paths.js:31:23:31:25 | obj | tainted-access-paths.js:31:23:31:30 | obj.sub4 |
70917103
| tainted-access-paths.js:31:23:31:25 | obj | tainted-access-paths.js:31:23:31:30 | obj.sub4 |
70927104
| tainted-require.js:7:19:7:37 | req.param("module") | tainted-require.js:7:19:7:37 | req.param("module") |
7105+
| tainted-require.js:12:29:12:47 | req.param("module") | tainted-require.js:12:29:12:47 | req.param("module") |
7106+
| tainted-require.js:14:11:14:29 | req.param("module") | tainted-require.js:14:11:14:29 | req.param("module") |
70937107
| tainted-sendFile.js:8:16:8:33 | req.param("gimme") | tainted-sendFile.js:8:16:8:33 | req.param("gimme") |
70947108
| tainted-sendFile.js:10:16:10:33 | req.param("gimme") | tainted-sendFile.js:10:16:10:33 | req.param("gimme") |
70957109
| 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
83048318
| 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 |
83058319
| 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 |
83068320
| 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 |
8321+
| 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 |
8322+
| 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 |
83078323
| 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 |
83088324
| 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 |
83098325
| 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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,12 @@ app.get('/some/path', function(req, res) {
66
// BAD: loading a module based on un-sanitized query parameters
77
var m = require(req.param("module"));
88
});
9+
10+
const resolve = require("resolve");
11+
app.get('/some/path', function(req, res) {
12+
var module = resolve.sync(req.param("module")); // NOT OK - resolving module based on query parameters
13+
14+
resolve(req.param("module"), { basedir: __dirname }, function(err, res) { // NOT OK - resolving module based on query parameters
15+
var module = res;
16+
});
17+
});

0 commit comments

Comments
 (0)