Skip to content

Commit 0f06f04

Browse files
erik-krogherik-krogh
committed
extend support for yargs for js/indirect-command-line-injection
1 parent d2d235d commit 0f06f04

3 files changed

Lines changed: 81 additions & 3 deletions

File tree

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

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,46 @@ module IndirectCommandInjection {
5050
// `require('minimist')(...)` => `{ _: [], a: ... b: ... }`
5151
this = DataFlow::moduleImport("minimist").getACall()
5252
or
53-
// `require('yargs').argv` => `{ _: [], a: ... b: ... }`
54-
this = DataFlow::moduleMember("yargs", "argv")
55-
or
5653
// `require('optimist').argv` => `{ _: [], a: ... b: ... }`
5754
this = DataFlow::moduleMember("optimist", "argv")
5855
}
5956
}
6057

58+
/**
59+
* Gets an instance of `yargs`.
60+
* Either directly imported as a module, or through some chained method call.
61+
*/
62+
private DataFlow::SourceNode yargs() {
63+
result = DataFlow::moduleImport("yargs")
64+
or
65+
result =
66+
// script used to generate list of chained methods: https://gist.github.com/erik-krogh/f8afe952c0577f4b563a993e613269ba
67+
yargs()
68+
.getAMethodCall(["middleware", "scriptName", "reset", "resetOptions", "boolean", "array",
69+
"number", "normalize", "count", "string", "requiresArg", "skipValidation", "nargs",
70+
"choices", "alias", "defaults", "default", "describe", "demandOption", "coerce",
71+
"config", "example", "require", "required", "demand", "demandCommand",
72+
"deprecateOption", "implies", "conflicts", "usage", "epilog", "epilogue", "fail",
73+
"onFinishCommand", "check", "global", "pkgConf", "options", "option", "positional",
74+
"group", "env", "wrap", "strict", "strictCommands", "parserConfiguration",
75+
"version", "help", "addHelpOpt", "showHidden", "addShowHiddenOpt", "hide",
76+
"showHelpOnFail", "exitProcess", "completion", "updateLocale", "updateStrings",
77+
"detectLocale", "recommendCommands", "getValidationInstance", "command",
78+
"commandDir", "showHelp", "showCompletionScript"])
79+
}
80+
81+
/**
82+
* An array of command line arguments (`argv`) parsed by the `yargs` libary.
83+
*/
84+
class YargsArgv extends Source {
85+
YargsArgv() {
86+
this = yargs().getAPropertyRead("argv")
87+
or
88+
this = yargs().getAMethodCall("parse") and
89+
this.(DataFlow::MethodCallNode).getNumArgument() = 0
90+
}
91+
}
92+
6193
/**
6294
* A command-line argument that effectively is system-controlled, and therefore not likely to be exploitable when used in the execution of another command.
6395
*/

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@ nodes
6868
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv |
6969
| command-line-parameter-command-injection.js:33:21:33:44 | require ... ").argv |
7070
| command-line-parameter-command-injection.js:33:21:33:48 | require ... rgv.foo |
71+
| command-line-parameter-command-injection.js:36:6:39:7 | args |
72+
| command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv |
73+
| command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv |
74+
| command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args |
75+
| command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args |
76+
| command-line-parameter-command-injection.js:41:22:41:25 | args |
77+
| command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
78+
| command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
79+
| command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() |
80+
| command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() |
81+
| command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo |
7182
edges
7283
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
7384
| 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] |
@@ -129,6 +140,15 @@ edges
129140
| 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 |
130141
| 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 |
131142
| 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 |
143+
| command-line-parameter-command-injection.js:36:6:39:7 | args | command-line-parameter-command-injection.js:41:22:41:25 | args |
144+
| command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line-parameter-command-injection.js:36:6:39:7 | args |
145+
| command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line-parameter-command-injection.js:36:6:39:7 | args |
146+
| command-line-parameter-command-injection.js:41:22:41:25 | args | command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args |
147+
| command-line-parameter-command-injection.js:41:22:41:25 | args | command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args |
148+
| command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo |
149+
| command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo |
150+
| command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
151+
| command-line-parameter-command-injection.js:43:22:43:62 | require ... e().foo | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo |
132152
#select
133153
| 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 |
134154
| 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 |
@@ -144,3 +164,5 @@ edges
144164
| 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 |
145165
| 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 |
146166
| 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 |
167+
| command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args | command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line-parameter-command-injection.js:41:10:41:25 | "cmd.sh " + args | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:36:13:39:7 | require ... \\t\\t.argv | command-line argument |
168+
| command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo | command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line-parameter-command-injection.js:43:10:43:62 | "cmd.sh ... e().foo | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:43:22:43:58 | require ... parse() | command-line argument |

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,27 @@ cp.exec("cmd.sh " + require("get-them-args")().foo); // NOT OK
3131
cp.exec("cmd.sh " + require("minimist")().foo); // NOT OK
3232
cp.exec("cmd.sh " + require("yargs").argv.foo); // NOT OK
3333
cp.exec("cmd.sh " + require("optimist").argv.foo); // NOT OK
34+
35+
(function () {
36+
var args = require('yargs') // eslint-disable-line
37+
.command('serve [port]', 'start the server', (yargs) => { })
38+
.option('verbose', { foo: "bar" })
39+
.argv
40+
41+
cp.exec("cmd.sh " + args); // NOT OK
42+
43+
cp.exec("cmd.sh " + require("yargs").array("foo").parse().foo); // NOT OK
44+
});
45+
46+
(function () {
47+
const {
48+
argv: {
49+
...args
50+
},
51+
} = yargs
52+
.usage('Usage: foo bar')
53+
.command();
54+
55+
cp.exec("cmd.sh " + args); // NOT OK - but not flagged yet.
56+
});
57+

0 commit comments

Comments
 (0)