Skip to content

Commit e8dc77d

Browse files
committed
add support for util.promisify with child_process calls
1 parent bfd80b4 commit e8dc77d

File tree

3 files changed

+33
-0
lines changed

3 files changed

+33
-0
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,14 @@ module NodeJSLib {
594594

595595
ChildProcessMethodCall() {
596596
this = DataFlow::moduleMember("child_process", methodName).getACall()
597+
or
598+
exists(DataFlow::CallNode promisify |
599+
promisify = DataFlow::moduleMember("util", "promisify").getACall()
600+
|
601+
this = promisify.getACall() and
602+
promisify.getArgument(0).getALocalSource() =
603+
DataFlow::moduleMember("child_process", methodName)
604+
)
597605
}
598606

599607
private DataFlow::Node getACommandArgument(boolean shell) {

javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ nodes
3939
| child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
4040
| child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
4141
| child_process-test.js:54:46:54:48 | cmd |
42+
| child_process-test.js:70:9:70:49 | cmd |
43+
| child_process-test.js:70:15:70:38 | url.par ... , true) |
44+
| child_process-test.js:70:15:70:44 | url.par ... ).query |
45+
| child_process-test.js:70:15:70:49 | url.par ... ry.path |
46+
| child_process-test.js:70:25:70:31 | req.url |
47+
| child_process-test.js:70:25:70:31 | req.url |
48+
| child_process-test.js:72:29:72:31 | cmd |
49+
| child_process-test.js:72:29:72:31 | cmd |
4250
| execSeries.js:3:20:3:22 | arr |
4351
| execSeries.js:6:14:6:16 | arr |
4452
| execSeries.js:6:14:6:21 | arr[i++] |
@@ -129,6 +137,13 @@ edges
129137
| child_process-test.js:53:54:53:56 | cmd | child_process-test.js:53:46:53:57 | ["bar", cmd] |
130138
| child_process-test.js:54:46:54:48 | cmd | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
131139
| child_process-test.js:54:46:54:48 | cmd | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
140+
| child_process-test.js:70:9:70:49 | cmd | child_process-test.js:72:29:72:31 | cmd |
141+
| child_process-test.js:70:9:70:49 | cmd | child_process-test.js:72:29:72:31 | cmd |
142+
| child_process-test.js:70:15:70:38 | url.par ... , true) | child_process-test.js:70:15:70:44 | url.par ... ).query |
143+
| child_process-test.js:70:15:70:44 | url.par ... ).query | child_process-test.js:70:15:70:49 | url.par ... ry.path |
144+
| child_process-test.js:70:15:70:49 | url.par ... ry.path | child_process-test.js:70:9:70:49 | cmd |
145+
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
146+
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
132147
| execSeries.js:3:20:3:22 | arr | execSeries.js:6:14:6:16 | arr |
133148
| execSeries.js:6:14:6:16 | arr | execSeries.js:6:14:6:21 | arr[i++] |
134149
| execSeries.js:6:14:6:21 | arr[i++] | execSeries.js:14:24:14:30 | command |
@@ -197,6 +212,7 @@ edges
197212
| child_process-test.js:54:5:54:50 | cp.spaw ... t(cmd)) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
198213
| child_process-test.js:59:5:59:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:50:15:50:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
199214
| child_process-test.js:64:3:64:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
215+
| child_process-test.js:72:29:72:31 | cmd | child_process-test.js:70:25:70:31 | req.url | child_process-test.js:72:29:72:31 | cmd | This command depends on $@. | child_process-test.js:70:25:70:31 | req.url | a user-provided value |
200216
| execSeries.js:14:41:14:47 | command | execSeries.js:18:34:18:40 | req.url | execSeries.js:14:41:14:47 | command | This command depends on $@. | execSeries.js:18:34:18:40 | req.url | a user-provided value |
201217
| other.js:7:33:7:35 | cmd | other.js:5:25:5:31 | req.url | other.js:7:33:7:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
202218
| other.js:8:28:8:30 | cmd | other.js:5:25:5:31 | req.url | other.js:8:28:8:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |

javascript/ql/test/query-tests/Security/CWE-078/child_process-test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,12 @@ var server = http.createServer(function(req, res) {
6363
function run(cmd, args) {
6464
cp.spawn(cmd, args); // NOT OK
6565
}
66+
67+
var util = require("util")
68+
69+
http.createServer(function(req, res) {
70+
let cmd = url.parse(req.url, true).query.path;
71+
72+
util.promisify(cp.exec)(cmd); // NOT OK
73+
});
74+

0 commit comments

Comments
 (0)