Skip to content

Commit 2f63b89

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #2338 from esbena/js/model-get-them-args
Approved by max-schaefer
2 parents 217eda3 + a6dbf5f commit 2f63b89

File tree

4 files changed

+66
-1
lines changed

4 files changed

+66
-1
lines changed

‎change-notes/1.23/analysis-javascript.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88

99
* Support for the following frameworks and libraries has been improved:
1010
- [firebase](https://www.npmjs.com/package/firebase)
11+
- [get-them-args](https://www.npmjs.com/package/get-them-args)
12+
- [minimist](https://www.npmjs.com/package/minimist)
1113
- [mongodb](https://www.npmjs.com/package/mongodb)
1214
- [mongoose](https://www.npmjs.com/package/mongoose)
13-
- [parse-torrent](https://github.com/webtorrent/parse-torrent)
15+
- [optimist](https://www.npmjs.com/package/optimist)
16+
- [parse-torrent](https://www.npmjs.com/package/parse-torrent)
1417
- [rate-limiter-flexible](https://www.npmjs.com/package/rate-limiter-flexible)
18+
- [yargs](https://www.npmjs.com/package/yargs)
1519

1620
* The call graph has been improved to resolve method calls in more cases. This may produce more security alerts.
1721

‎javascript/ql/src/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,22 @@ module IndirectCommandInjection {
3939
}
4040
}
4141

42+
/**
43+
* An object containing parsed command-line arguments, considered as a flow source for command injection.
44+
*/
45+
class ParsedCommandLineArgumentsAsSource extends Source {
46+
ParsedCommandLineArgumentsAsSource() {
47+
// `require('get-them-args')(...)` => `{ unknown: [], a: ... b: ... }`
48+
this = DataFlow::moduleImport("get-them-args").getACall() or
49+
// `require('minimist')(...)` => `{ _: [], a: ... b: ... }`
50+
this = DataFlow::moduleImport("minimist").getACall() or
51+
// `require('yargs').argv` => `{ _: [], a: ... b: ... }`
52+
this = DataFlow::moduleMember("yargs", "argv") or
53+
// `require('optimist').argv` => `{ _: [], a: ... b: ... }`
54+
this = DataFlow::moduleMember("optimist", "argv")
55+
}
56+
}
57+
4258
/**
4359
* A command-line argument that effectively is system-controlled, and therefore not likely to be exploitable when used in the execution of another command.
4460
*/

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,26 @@ nodes
4848
| command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
4949
| command-line-parameter-command-injection.js:27:32:27:35 | args |
5050
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') |
51+
| command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo |
52+
| command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo |
53+
| command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() |
54+
| command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() |
55+
| command-line-parameter-command-injection.js:30:21:30:50 | require ... )().foo |
56+
| command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo |
57+
| command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo |
58+
| command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() |
59+
| command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() |
60+
| command-line-parameter-command-injection.js:31:21:31:45 | require ... )().foo |
61+
| command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo |
62+
| command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo |
63+
| command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv |
64+
| command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv |
65+
| command-line-parameter-command-injection.js:32:21:32:45 | require ... rgv.foo |
66+
| command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo |
67+
| command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo |
68+
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv |
69+
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv |
70+
| command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo |
5171
edges
5272
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
5373
| command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
@@ -93,6 +113,22 @@ edges
93113
| command-line-parameter-command-injection.js:27:32:27:35 | args | command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') |
94114
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
95115
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
116+
| command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() | command-line-parameter-command-injection.js:30:21:30:50 | require ... )().foo |
117+
| command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() | command-line-parameter-command-injection.js:30:21:30:50 | require ... )().foo |
118+
| command-line-parameter-command-injection.js:30:21:30:50 | require ... )().foo | command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo |
119+
| command-line-parameter-command-injection.js:30:21:30:50 | require ... )().foo | command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo |
120+
| command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() | command-line-parameter-command-injection.js:31:21:31:45 | require ... )().foo |
121+
| command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() | command-line-parameter-command-injection.js:31:21:31:45 | require ... )().foo |
122+
| command-line-parameter-command-injection.js:31:21:31:45 | require ... )().foo | command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo |
123+
| command-line-parameter-command-injection.js:31:21:31:45 | require ... )().foo | command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo |
124+
| command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv | command-line-parameter-command-injection.js:32:21:32:45 | require ... rgv.foo |
125+
| command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv | command-line-parameter-command-injection.js:32:21:32:45 | require ... rgv.foo |
126+
| command-line-parameter-command-injection.js:32:21:32:45 | require ... rgv.foo | command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo |
127+
| command-line-parameter-command-injection.js:32:21:32:45 | require ... rgv.foo | command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo |
128+
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv | command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo |
129+
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv | command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo |
130+
| command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo | command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo |
131+
| command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo | command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo |
96132
#select
97133
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument |
98134
| command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument |
@@ -104,3 +140,7 @@ edges
104140
| command-line-parameter-command-injection.js:20:14:20:29 | "cmd.sh " + arg0 | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line-parameter-command-injection.js:20:14:20:29 | "cmd.sh " + arg0 | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line argument |
105141
| command-line-parameter-command-injection.js:26:14:26:50 | `node $ ... ption"` | command-line-parameter-command-injection.js:24:15:24:26 | process.argv | command-line-parameter-command-injection.js:26:14:26:50 | `node $ ... ption"` | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:24:15:24:26 | process.argv | command-line argument |
106142
| command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` | command-line-parameter-command-injection.js:24:15:24:26 | process.argv | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:24:15:24:26 | process.argv | command-line argument |
143+
| command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo | command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() | command-line-parameter-command-injection.js:30:9:30:50 | "cmd.sh ... )().foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:30:21:30:46 | require ... rgs")() | command-line argument |
144+
| command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo | command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() | command-line-parameter-command-injection.js:31:9:31:45 | "cmd.sh ... )().foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:31:21:31:41 | require ... ist")() | command-line argument |
145+
| command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo | command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv | command-line-parameter-command-injection.js:32:9:32:45 | "cmd.sh ... rgv.foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:32:21:32:41 | require ... ").argv | command-line argument |
146+
| command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo | command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv | command-line-parameter-command-injection.js:33:9:33:48 | "cmd.sh ... rgv.foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv | command-line argument |

‎javascript/ql/test/query-tests/Security/CWE-078/command-line-parameter-command-injection.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,8 @@ var cp = require("child_process");
2626
cp.execSync(`node ${script} ${args[0]} --option"`); // NOT OK
2727
cp.execSync(`node ${script} ${args.join(' ')} --option"`); // NOT OK
2828
});
29+
30+
cp.exec("cmd.sh " + require("get-them-args")().foo); // NOT OK
31+
cp.exec("cmd.sh " + require("minimist")().foo); // NOT OK
32+
cp.exec("cmd.sh " + require("yargs").argv.foo); // NOT OK
33+
cp.exec("cmd.sh " + require("optimist").argv.foo); // NOT OK

0 commit comments

Comments
 (0)