Skip to content

Commit 41ef7a3

Browse files
authored
Merge pull request #4733 from erik-krogh/args
Approved by esbena
2 parents 287954e + 94e07bb commit 41ef7a3

File tree

5 files changed

+345
-14
lines changed

5 files changed

+345
-14
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
lgtm,codescanning
2+
* The `js/indirect-command-line-injection` query now supports more command-line parsing libraries.
3+
Affected packages are
4+
[arg](https://www.npmjs.com/package/arg),
5+
[argparse](https://www.npmjs.com/package/argparse),
6+
[command-line-args](https://www.npmjs.com/package/command-line-args),
7+
[meow](https://www.npmjs.com/package/meow),
8+
[dashdash](https://www.npmjs.com/package/dashdash),
9+
[commander](https://www.npmjs.com/package/commander).

javascript/ql/src/semmle/javascript/security/dataflow/IndirectCommandInjection.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,9 @@ module IndirectCommandInjection {
3030
override predicate isSink(DataFlow::Node sink) { isSinkWithHighlight(sink, _) }
3131

3232
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
33+
34+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
35+
argsParseStep(pred, succ)
36+
}
3337
}
3438
}

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

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,70 @@ module IndirectCommandInjection {
4747
// `require('get-them-args')(...)` => `{ unknown: [], a: ... b: ... }`
4848
this = DataFlow::moduleImport("get-them-args").getACall()
4949
or
50-
// `require('minimist')(...)` => `{ _: [], a: ... b: ... }`
51-
this = DataFlow::moduleImport("minimist").getACall()
52-
or
5350
// `require('optimist').argv` => `{ _: [], a: ... b: ... }`
5451
this = DataFlow::moduleMember("optimist", "argv")
52+
or
53+
// `require("arg")({...spec})` => `{_: [], a: ..., b: ...}`
54+
this = DataFlow::moduleImport("arg").getACall()
55+
or
56+
// `(new (require(argparse)).ArgumentParser({...spec})).parse_args()` => `{a: ..., b: ...}`
57+
this =
58+
API::moduleImport("argparse")
59+
.getMember("ArgumentParser")
60+
.getInstance()
61+
.getMember("parse_args")
62+
.getACall()
63+
or
64+
// `require('command-line-args')({...spec})` => `{a: ..., b: ...}`
65+
this = DataFlow::moduleImport("command-line-args").getACall()
66+
or
67+
// `require('meow')(help, {...spec})` => `{a: ..., b: ....}`
68+
this = DataFlow::moduleImport("meow").getACall()
69+
or
70+
// `require("dashdash").createParser(...spec)` => `{a: ..., b: ...}`
71+
this =
72+
[
73+
API::moduleImport("dashdash"),
74+
API::moduleImport("dashdash").getMember("createParser").getReturn()
75+
].getMember("parse").getACall()
76+
or
77+
// `require('commander').myCmdArgumentName`
78+
this = commander().getAMember().getAnImmediateUse()
79+
or
80+
// `require('commander').opt()` => `{a: ..., b: ...}`
81+
this = commander().getMember("opts").getACall()
5582
}
5683
}
5784

85+
/**
86+
* A command line parsing step from `pred` to `succ`.
87+
* E.g: `var succ = require("minimist")(pred)`.
88+
*/
89+
predicate argsParseStep(DataFlow::Node pred, DataFlow::Node succ) {
90+
exists(DataFlow::CallNode call |
91+
call = DataFlow::moduleMember("args", "parse").getACall() or
92+
call = DataFlow::moduleImport(["yargs-parser", "minimist", "subarg"]).getACall()
93+
|
94+
succ = call and
95+
pred = call.getArgument(0)
96+
)
97+
}
98+
99+
/**
100+
* A Command instance from the `commander` library.
101+
*/
102+
private API::Node commander() {
103+
result = API::moduleImport("commander")
104+
or
105+
// `require("commander").program === require("commander")`
106+
result = commander().getMember("program")
107+
or
108+
result = commander().getMember("Command").getInstance()
109+
or
110+
// lots of chainable methods
111+
result = commander().getAMember().getReturn()
112+
}
113+
58114
/**
59115
* Gets an instance of `yargs`.
60116
* Either directly imported as a module, or through some chained method call.

0 commit comments

Comments
 (0)