Skip to content

Commit e980798

Browse files
committed
Added step through yargs/yargs constructor and chained methods.
1 parent e8eb9be commit e980798

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

javascript/ql/lib/semmle/javascript/frameworks/CommandLineArguments.qll

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,18 @@ private class ArgsParseStep extends TaintTracking::SharedTaintStep {
8787
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
8888
exists(DataFlow::CallNode call |
8989
call = DataFlow::moduleMember("args", "parse").getACall() or
90-
call = DataFlow::moduleImport(["yargs-parser", "minimist", "subarg"]).getACall()
90+
call =
91+
DataFlow::moduleImport(["yargs-parser", "minimist", "subarg", "yargs/yargs", "yargs"])
92+
.getACall()
9193
|
9294
succ = call and
9395
pred = call.getArgument(0)
9496
)
97+
or
98+
exists(DataFlow::MethodCallNode methodCall | methodCall = yargs() |
99+
pred = methodCall.getReceiver() and
100+
succ = methodCall
101+
)
95102
}
96103
}
97104

@@ -115,7 +122,9 @@ private API::Node commander() {
115122
* Either directly imported as a module, or through some chained method call.
116123
*/
117124
private DataFlow::SourceNode yargs() {
118-
result = DataFlow::moduleImport("yargs")
125+
result = DataFlow::moduleImport(["yargs", "yargs/yargs"])
126+
or
127+
result = DataFlow::moduleImport(["yargs", "yargs/yargs"]).getACall()
119128
or
120129
// script used to generate list of chained methods: https://gist.github.com/erik-krogh/f8afe952c0577f4b563a993e613269ba
121130
exists(string method |

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
| 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 line depends on a $@. | child_process-test.js:73:25:73:31 | req.url | user-provided value |
2222
| 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 line depends on a $@. | child_process-test.js:83:19:83:36 | req.query.fileName | user-provided value |
2323
| 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 line depends on a $@. | child_process-test.js:94:21:94:30 | ctx.params | user-provided value |
24+
| command-line-libs.js:40:8:40:17 | parsed.cmd | command-line-libs.js:33:16:33:23 | req.body | command-line-libs.js:40:8:40:17 | parsed.cmd | This command line depends on a $@. | command-line-libs.js:33:16:33:23 | req.body | user-provided value |
2425
| 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 line depends on a $@. | exec-sh2.js:14:25:14:31 | req.url | user-provided value |
2526
| 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 line depends on a $@. | exec-sh.js:19:25:19:31 | req.url | user-provided value |
2627
| execSeries.js:14:41:14:47 | command | execSeries.js:18:34:18:40 | req.url | execSeries.js:14:41:14:47 | command | This command line depends on a $@. | execSeries.js:18:34:18:40 | req.url | user-provided value |
@@ -116,6 +117,14 @@ edges
116117
| child_process-test.js:73:15:73:38 | url.par ... , true) | child_process-test.js:73:9:73:49 | cmd | provenance | |
117118
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) | provenance | |
118119
| child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:11:94:35 | "ping " ... ms.host | provenance | |
120+
| command-line-libs.js:33:9:33:34 | args | command-line-libs.js:34:24:34:27 | args | provenance | |
121+
| command-line-libs.js:33:16:33:23 | req.body | command-line-libs.js:33:9:33:34 | args | provenance | |
122+
| command-line-libs.js:34:9:38:12 | parsed | command-line-libs.js:40:8:40:13 | parsed | provenance | |
123+
| command-line-libs.js:34:18:34:28 | yargs(args) | command-line-libs.js:34:18:38:4 | yargs(a ... ue\\n }) | provenance | |
124+
| command-line-libs.js:34:18:38:4 | yargs(a ... ue\\n }) | command-line-libs.js:34:18:38:12 | yargs(a ... parse() | provenance | |
125+
| command-line-libs.js:34:18:38:12 | yargs(a ... parse() | command-line-libs.js:34:9:38:12 | parsed | provenance | |
126+
| command-line-libs.js:34:24:34:27 | args | command-line-libs.js:34:18:34:28 | yargs(args) | provenance | |
127+
| command-line-libs.js:40:8:40:13 | parsed | command-line-libs.js:40:8:40:17 | parsed.cmd | provenance | |
119128
| exec-sh2.js:9:17:9:23 | command | exec-sh2.js:10:40:10:46 | command | provenance | |
120129
| exec-sh2.js:14:9:14:49 | cmd | exec-sh2.js:15:12:15:14 | cmd | provenance | |
121130
| exec-sh2.js:14:15:14:38 | url.par ... , true) | exec-sh2.js:14:9:14:49 | cmd | provenance | |
@@ -269,6 +278,15 @@ nodes
269278
| child_process-test.js:83:19:83:36 | req.query.fileName | semmle.label | req.query.fileName |
270279
| child_process-test.js:94:11:94:35 | "ping " ... ms.host | semmle.label | "ping " ... ms.host |
271280
| child_process-test.js:94:21:94:30 | ctx.params | semmle.label | ctx.params |
281+
| command-line-libs.js:33:9:33:34 | args | semmle.label | args |
282+
| command-line-libs.js:33:16:33:23 | req.body | semmle.label | req.body |
283+
| command-line-libs.js:34:9:38:12 | parsed | semmle.label | parsed |
284+
| command-line-libs.js:34:18:34:28 | yargs(args) | semmle.label | yargs(args) |
285+
| command-line-libs.js:34:18:38:4 | yargs(a ... ue\\n }) | semmle.label | yargs(a ... ue\\n }) |
286+
| command-line-libs.js:34:18:38:12 | yargs(a ... parse() | semmle.label | yargs(a ... parse() |
287+
| command-line-libs.js:34:24:34:27 | args | semmle.label | args |
288+
| command-line-libs.js:40:8:40:13 | parsed | semmle.label | parsed |
289+
| command-line-libs.js:40:8:40:17 | parsed.cmd | semmle.label | parsed.cmd |
272290
| exec-sh2.js:9:17:9:23 | command | semmle.label | command |
273291
| exec-sh2.js:10:40:10:46 | command | semmle.label | command |
274292
| exec-sh2.js:14:9:14:49 | cmd | semmle.label | cmd |

javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/command-line-libs.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ app.post('/commandLineArgs', (req, res) => {
3030

3131
app.post('/yargs', (req, res) => {
3232
const yargs = require('yargs/yargs');
33-
const args = req.body.args || []; // $ MISSING: Source
33+
const args = req.body.args || []; // $ Source
3434
const parsed = yargs(args).option('cmd', {
3535
type: 'string',
3636
describe: 'Command to execute',
3737
demandOption: true
3838
}).parse();
3939

40-
exec(parsed.cmd); // $ MISSING: Alert
40+
exec(parsed.cmd); // $ Alert
4141
});

0 commit comments

Comments
 (0)