Skip to content

Commit b19d1e0

Browse files
authored
Merge pull request #20151 from Napalys/js/command-line-libs
JS: Enhance command injection detection for CLI argument parsing libraries
2 parents b234618 + 881ea76 commit b19d1e0

File tree

4 files changed

+157
-2
lines changed

4 files changed

+157
-2
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improved modeling of command-line argument parsing libraries [arg](https://www.npmjs.com/package/arg), [args](https://www.npmjs.com/package/args), [command-line-args](https://www.npmjs.com/package/command-line-args) and [commander](https://www.npmjs.com/package/commander)

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,43 @@ 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(API::Node commanderNode | commanderNode = commander() |
99+
pred = commanderNode.getMember(["parse", "parseAsync"]).getACall().getAnArgument() and
100+
succ =
101+
[
102+
commanderNode.getMember("opts").getACall(), commanderNode.getAMember().asSource(),
103+
commander()
104+
.getMember("action")
105+
.getACall()
106+
.getArgument(0)
107+
.(DataFlow::FunctionNode)
108+
.getAParameter()
109+
]
110+
)
111+
or
112+
exists(DataFlow::MethodCallNode methodCall | methodCall = yargs() |
113+
pred = methodCall.getReceiver() and
114+
succ = methodCall
115+
)
116+
or
117+
exists(DataFlow::CallNode call, DataFlow::Node options |
118+
call = DataFlow::moduleImport(["arg", "command-line-args"]).getACall() and
119+
succ = call and
120+
options = call.getArgument(1) and
121+
exists(DataFlow::PropWrite write |
122+
write.getBase() = options and
123+
write.getPropertyName() = "argv" and
124+
pred = write.getRhs()
125+
)
126+
)
95127
}
96128
}
97129

@@ -115,7 +147,9 @@ private API::Node commander() {
115147
* Either directly imported as a module, or through some chained method call.
116148
*/
117149
private DataFlow::SourceNode yargs() {
118-
result = DataFlow::moduleImport("yargs")
150+
result = DataFlow::moduleImport(["yargs", "yargs/yargs"])
151+
or
152+
result = DataFlow::moduleImport(["yargs", "yargs/yargs"]).getACall()
119153
or
120154
// script used to generate list of chained methods: https://gist.github.com/erik-krogh/f8afe952c0577f4b563a993e613269ba
121155
exists(string method |

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
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:14:8:14:18 | options.cmd | command-line-libs.js:9:16:9:23 | req.body | command-line-libs.js:14:8:14:18 | options.cmd | This command line depends on a $@. | command-line-libs.js:9:16:9:23 | req.body | user-provided value |
25+
| command-line-libs.js:15:8:15:18 | program.cmd | command-line-libs.js:9:16:9:23 | req.body | command-line-libs.js:15:8:15:18 | program.cmd | This command line depends on a $@. | command-line-libs.js:9:16:9:23 | req.body | user-provided value |
26+
| command-line-libs.js:21:12:21:17 | script | command-line-libs.js:9:16:9:23 | req.body | command-line-libs.js:21:12:21:17 | script | This command line depends on a $@. | command-line-libs.js:9:16:9:23 | req.body | user-provided value |
27+
| command-line-libs.js:29:10:29:24 | parsed['--cmd'] | command-line-libs.js:27:23:27:30 | req.body | command-line-libs.js:29:10:29:24 | parsed['--cmd'] | This command line depends on a $@. | command-line-libs.js:27:23:27:30 | req.body | user-provided value |
28+
| command-line-libs.js:37:8:37:18 | options.cmd | command-line-libs.js:35:62:35:69 | req.body | command-line-libs.js:37:8:37:18 | options.cmd | This command line depends on a $@. | command-line-libs.js:35:62:35:69 | req.body | user-provided value |
29+
| command-line-libs.js:49:8:49:17 | parsed.cmd | command-line-libs.js:42:16:42:23 | req.body | command-line-libs.js:49:8:49:17 | parsed.cmd | This command line depends on a $@. | command-line-libs.js:42:16:42:23 | req.body | user-provided value |
2430
| 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 |
2531
| 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 |
2632
| 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 +122,35 @@ edges
116122
| child_process-test.js:73:15:73:38 | url.par ... , true) | child_process-test.js:73:9:73:49 | cmd | provenance | |
117123
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) | provenance | |
118124
| child_process-test.js:94:21:94:30 | ctx.params | child_process-test.js:94:11:94:35 | "ping " ... ms.host | provenance | |
125+
| command-line-libs.js:9:9:9:34 | args | command-line-libs.js:12:17:12:20 | args | provenance | |
126+
| command-line-libs.js:9:9:9:34 | args | command-line-libs.js:23:29:23:32 | args | provenance | |
127+
| command-line-libs.js:9:16:9:23 | req.body | command-line-libs.js:9:9:9:34 | args | provenance | |
128+
| command-line-libs.js:12:17:12:20 | args | command-line-libs.js:13:19:13:32 | program.opts() | provenance | |
129+
| command-line-libs.js:12:17:12:20 | args | command-line-libs.js:15:8:15:18 | program.cmd | provenance | |
130+
| command-line-libs.js:12:17:12:20 | args | command-line-libs.js:20:14:20:19 | script | provenance | |
131+
| command-line-libs.js:13:9:13:32 | options | command-line-libs.js:14:8:14:14 | options | provenance | |
132+
| command-line-libs.js:13:19:13:32 | program.opts() | command-line-libs.js:13:9:13:32 | options | provenance | |
133+
| command-line-libs.js:14:8:14:14 | options | command-line-libs.js:14:8:14:18 | options.cmd | provenance | |
134+
| command-line-libs.js:20:14:20:19 | script | command-line-libs.js:21:12:21:17 | script | provenance | |
135+
| command-line-libs.js:23:29:23:32 | args | command-line-libs.js:20:14:20:19 | script | provenance | |
136+
| command-line-libs.js:27:11:27:41 | argsArray | command-line-libs.js:28:53:28:61 | argsArray | provenance | |
137+
| command-line-libs.js:27:23:27:30 | req.body | command-line-libs.js:27:11:27:41 | argsArray | provenance | |
138+
| command-line-libs.js:28:11:28:64 | parsed | command-line-libs.js:29:10:29:15 | parsed | provenance | |
139+
| command-line-libs.js:28:20:28:64 | arg({ ' ... rray }) | command-line-libs.js:28:11:28:64 | parsed | provenance | |
140+
| command-line-libs.js:28:53:28:61 | argsArray | command-line-libs.js:28:20:28:64 | arg({ ' ... rray }) | provenance | |
141+
| command-line-libs.js:29:10:29:15 | parsed | command-line-libs.js:29:10:29:24 | parsed['--cmd'] | provenance | |
142+
| command-line-libs.js:35:9:35:83 | options | command-line-libs.js:37:8:37:14 | options | provenance | |
143+
| command-line-libs.js:35:19:35:83 | command ... \| [] }) | command-line-libs.js:35:9:35:83 | options | provenance | |
144+
| command-line-libs.js:35:62:35:69 | req.body | command-line-libs.js:35:19:35:83 | command ... \| [] }) | provenance | |
145+
| command-line-libs.js:37:8:37:14 | options | command-line-libs.js:37:8:37:18 | options.cmd | provenance | |
146+
| command-line-libs.js:42:9:42:34 | args | command-line-libs.js:43:24:43:27 | args | provenance | |
147+
| command-line-libs.js:42:16:42:23 | req.body | command-line-libs.js:42:9:42:34 | args | provenance | |
148+
| command-line-libs.js:43:9:47:12 | parsed | command-line-libs.js:49:8:49:13 | parsed | provenance | |
149+
| command-line-libs.js:43:18:43:28 | yargs(args) | command-line-libs.js:43:18:47:4 | yargs(a ... ue\\n }) | provenance | |
150+
| command-line-libs.js:43:18:47:4 | yargs(a ... ue\\n }) | command-line-libs.js:43:18:47:12 | yargs(a ... parse() | provenance | |
151+
| command-line-libs.js:43:18:47:12 | yargs(a ... parse() | command-line-libs.js:43:9:47:12 | parsed | provenance | |
152+
| command-line-libs.js:43:24:43:27 | args | command-line-libs.js:43:18:43:28 | yargs(args) | provenance | |
153+
| command-line-libs.js:49:8:49:13 | parsed | command-line-libs.js:49:8:49:17 | parsed.cmd | provenance | |
119154
| exec-sh2.js:9:17:9:23 | command | exec-sh2.js:10:40:10:46 | command | provenance | |
120155
| exec-sh2.js:14:9:14:49 | cmd | exec-sh2.js:15:12:15:14 | cmd | provenance | |
121156
| exec-sh2.js:14:15:14:38 | url.par ... , true) | exec-sh2.js:14:9:14:49 | cmd | provenance | |
@@ -269,6 +304,38 @@ nodes
269304
| child_process-test.js:83:19:83:36 | req.query.fileName | semmle.label | req.query.fileName |
270305
| child_process-test.js:94:11:94:35 | "ping " ... ms.host | semmle.label | "ping " ... ms.host |
271306
| child_process-test.js:94:21:94:30 | ctx.params | semmle.label | ctx.params |
307+
| command-line-libs.js:9:9:9:34 | args | semmle.label | args |
308+
| command-line-libs.js:9:16:9:23 | req.body | semmle.label | req.body |
309+
| command-line-libs.js:12:17:12:20 | args | semmle.label | args |
310+
| command-line-libs.js:13:9:13:32 | options | semmle.label | options |
311+
| command-line-libs.js:13:19:13:32 | program.opts() | semmle.label | program.opts() |
312+
| command-line-libs.js:14:8:14:14 | options | semmle.label | options |
313+
| command-line-libs.js:14:8:14:18 | options.cmd | semmle.label | options.cmd |
314+
| command-line-libs.js:15:8:15:18 | program.cmd | semmle.label | program.cmd |
315+
| command-line-libs.js:20:14:20:19 | script | semmle.label | script |
316+
| command-line-libs.js:21:12:21:17 | script | semmle.label | script |
317+
| command-line-libs.js:23:29:23:32 | args | semmle.label | args |
318+
| command-line-libs.js:27:11:27:41 | argsArray | semmle.label | argsArray |
319+
| command-line-libs.js:27:23:27:30 | req.body | semmle.label | req.body |
320+
| command-line-libs.js:28:11:28:64 | parsed | semmle.label | parsed |
321+
| command-line-libs.js:28:20:28:64 | arg({ ' ... rray }) | semmle.label | arg({ ' ... rray }) |
322+
| command-line-libs.js:28:53:28:61 | argsArray | semmle.label | argsArray |
323+
| command-line-libs.js:29:10:29:15 | parsed | semmle.label | parsed |
324+
| command-line-libs.js:29:10:29:24 | parsed['--cmd'] | semmle.label | parsed['--cmd'] |
325+
| command-line-libs.js:35:9:35:83 | options | semmle.label | options |
326+
| command-line-libs.js:35:19:35:83 | command ... \| [] }) | semmle.label | command ... \| [] }) |
327+
| command-line-libs.js:35:62:35:69 | req.body | semmle.label | req.body |
328+
| command-line-libs.js:37:8:37:14 | options | semmle.label | options |
329+
| command-line-libs.js:37:8:37:18 | options.cmd | semmle.label | options.cmd |
330+
| command-line-libs.js:42:9:42:34 | args | semmle.label | args |
331+
| command-line-libs.js:42:16:42:23 | req.body | semmle.label | req.body |
332+
| command-line-libs.js:43:9:47:12 | parsed | semmle.label | parsed |
333+
| command-line-libs.js:43:18:43:28 | yargs(args) | semmle.label | yargs(args) |
334+
| command-line-libs.js:43:18:47:4 | yargs(a ... ue\\n }) | semmle.label | yargs(a ... ue\\n }) |
335+
| command-line-libs.js:43:18:47:12 | yargs(a ... parse() | semmle.label | yargs(a ... parse() |
336+
| command-line-libs.js:43:24:43:27 | args | semmle.label | args |
337+
| command-line-libs.js:49:8:49:13 | parsed | semmle.label | parsed |
338+
| command-line-libs.js:49:8:49:17 | parsed.cmd | semmle.label | parsed.cmd |
272339
| exec-sh2.js:9:17:9:23 | command | semmle.label | command |
273340
| exec-sh2.js:10:40:10:46 | command | semmle.label | command |
274341
| exec-sh2.js:14:9:14:49 | cmd | semmle.label | cmd |
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import express from 'express';
2+
import { Command } from 'commander';
3+
import { exec } from 'child_process';
4+
import arg from 'arg';
5+
const app = express();
6+
app.use(express.json());
7+
8+
app.post('/Command', async (req, res) => {
9+
const args = req.body.args || []; // $ Source
10+
const program = new Command();
11+
program.option('--cmd <value>', 'Command to execute');
12+
program.parse(args, { from: 'user' });
13+
const options = program.opts();
14+
exec(options.cmd); // $ Alert
15+
exec(program.cmd); // $ Alert
16+
17+
const program1 = new Command();
18+
program1
19+
.command('run <script>')
20+
.action((script) => {
21+
exec(script); // $ Alert
22+
});
23+
await program1.parseAsync(args);
24+
});
25+
26+
app.post('/arg', (req, res) => {
27+
const argsArray = req.body.args || []; // $ Source
28+
const parsed = arg({ '--cmd': String }, { argv: argsArray });
29+
exec(parsed['--cmd']); // $ Alert
30+
});
31+
32+
app.post('/commandLineArgs', (req, res) => {
33+
const commandLineArgs = require('command-line-args');
34+
const optionDefinitions = [{ name: 'cmd', type: String }];
35+
const options = commandLineArgs(optionDefinitions, { argv: req.body.args || [] }); // $ Source
36+
if (!options.cmd) return res.status(400).send({ error: 'Missing --cmd' });
37+
exec(options.cmd); // $ Alert
38+
});
39+
40+
app.post('/yargs', (req, res) => {
41+
const yargs = require('yargs/yargs');
42+
const args = req.body.args || []; // $ Source
43+
const parsed = yargs(args).option('cmd', {
44+
type: 'string',
45+
describe: 'Command to execute',
46+
demandOption: true
47+
}).parse();
48+
49+
exec(parsed.cmd); // $ Alert
50+
});

0 commit comments

Comments
 (0)