Skip to content

Commit d316ad1

Browse files
authored
Merge pull request #8380 from erik-krogh/chainedCalls
JS: support that the base is not a method-call in getAChainedMethodCall
2 parents 19c7f7b + 4177832 commit d316ad1

File tree

3 files changed

+19
-1
lines changed

3 files changed

+19
-1
lines changed

javascript/ql/lib/semmle/javascript/dataflow/Sources.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ class SourceNode extends DataFlow::Node {
146146
* that is, `o.m(...)` or `o[p](...)`.
147147
*/
148148
DataFlow::CallNode getAChainedMethodCall(string methodName) {
149-
result = getAMethodCall*().getAMethodCall(methodName)
149+
// the direct call to `getAMethodCall` is needed in case the base is not a `DataFlow::CallNode`.
150+
result = [getAMethodCall+().getAMethodCall(methodName), getAMethodCall(methodName)]
150151
}
151152

152153
/**

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ nodes
5252
| child_process-test.js:83:19:83:36 | req.query.fileName |
5353
| child_process-test.js:85:37:85:54 | req.query.fileName |
5454
| child_process-test.js:85:37:85:54 | req.query.fileName |
55+
| child_process-test.js:94:11:94:35 | "ping " ... ms.host |
56+
| child_process-test.js:94:11:94:35 | "ping " ... ms.host |
57+
| child_process-test.js:94:21:94:30 | ctx.params |
58+
| child_process-test.js:94:21:94:30 | ctx.params |
59+
| child_process-test.js:94:21:94:35 | ctx.params.host |
5560
| exec-sh2.js:9:17:9:23 | command |
5661
| exec-sh2.js:10:40:10:46 | command |
5762
| exec-sh2.js:10:40:10:46 | command |
@@ -229,6 +234,10 @@ edges
229234
| child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName |
230235
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
231236
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
237+
| child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:21:94:35 | ctx.params.host |
238+
| child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:21:94:35 | ctx.params.host |
239+
| child_process-test.js:94:21:94:35 | ctx.params.host | child_process-test.js:94:11:94:35 | "ping " ... ms.host |
240+
| child_process-test.js:94:21:94:35 | ctx.params.host | child_process-test.js:94:11:94:35 | "ping " ... ms.host |
232241
| exec-sh2.js:9:17:9:23 | command | exec-sh2.js:10:40:10:46 | command |
233242
| exec-sh2.js:9:17:9:23 | command | exec-sh2.js:10:40:10:46 | command |
234243
| exec-sh2.js:14:9:14:49 | cmd | exec-sh2.js:15:12:15:14 | cmd |
@@ -365,6 +374,7 @@ edges
365374
| child_process-test.js:67:3:67:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:48:15:48:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
366375
| child_process-test.js:75:29:75:31 | cmd | child_process-test.js:73:25:73:31 | req.url | child_process-test.js:75:29:75:31 | cmd | This command depends on $@. | child_process-test.js:73:25:73:31 | req.url | a user-provided value |
367376
| child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName | This command depends on $@. | child_process-test.js:83:19:83:36 | req.query.fileName | a user-provided value |
377+
| child_process-test.js:94:11:94:35 | "ping " ... ms.host | child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:11:94:35 | "ping " ... ms.host | This command depends on $@. | child_process-test.js:94:21:94:30 | ctx.params | a user-provided value |
368378
| exec-sh2.js:10:12:10:57 | cp.spaw ... ptions) | exec-sh2.js:14:25:14:31 | req.url | exec-sh2.js:10:40:10:46 | command | This command depends on $@. | exec-sh2.js:14:25:14:31 | req.url | a user-provided value |
369379
| exec-sh.js:15:12:15:61 | cp.spaw ... ptions) | exec-sh.js:19:25:19:31 | req.url | exec-sh.js:15:44:15:50 | command | This command depends on $@. | exec-sh.js:19:25:19:31 | req.url | a user-provided value |
370380
| 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 |

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,11 @@ new webpackDevServer(compiler, {
8585
require("my-sub-lib").foo(req.query.fileName); // calls lib/subLib/index.js#foo
8686
});
8787
}
88+
});
89+
90+
import Router from "koa-router";
91+
const router = new Router();
92+
93+
router.get("/ping/:host", async (ctx) => {
94+
cp.exec("ping " + ctx.params.host); // NOT OK
8895
});

0 commit comments

Comments
 (0)