mirror of
https://github.com/github/codeql.git
synced 2026-05-02 04:05:14 +02:00
add shell interpreted arrays as sinks for js/shell-command-constructed-from-input
This commit is contained in:
Erik Krogh Kristensen
@@ -143,6 +143,53 @@ module UnsafeShellCommandConstruction {
|
||||
override DataFlow::Node getAlertLocation() { result = this }
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a node that ends up in an array that is ultimately executed as a shell script by `sys`.
|
||||
*/
|
||||
private DataFlow::SourceNode endsInShellExecutedArray(
|
||||
DataFlow::TypeBackTracker t, SystemCommandExecution sys
|
||||
) {
|
||||
t.start() and
|
||||
result = sys.getArgumentList().getALocalSource() and
|
||||
// the array gets joined to a string when `shell` is set to true.
|
||||
sys.getOptionsArg()
|
||||
.getALocalSource()
|
||||
.getAPropertyWrite("shell")
|
||||
.getRhs()
|
||||
.asExpr()
|
||||
.(BooleanLiteral)
|
||||
.getValue() = "true"
|
||||
or
|
||||
exists(DataFlow::TypeBackTracker t2 |
|
||||
result = endsInShellExecutedArray(t2, sys).backtrack(t2, t)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* An argument to a command invocation where the `shell` option is set to true.
|
||||
*/
|
||||
class ShellTrueCommandExecutionSink extends Sink {
|
||||
SystemCommandExecution sys;
|
||||
|
||||
ShellTrueCommandExecutionSink() {
|
||||
// `shell` is set to true. That means string-concatenation happens behind the scenes.
|
||||
// We just assume that a `shell` option in any library means the same thing as it does in NodeJS.
|
||||
exists(DataFlow::SourceNode arr |
|
||||
arr = endsInShellExecutedArray(DataFlow::TypeBackTracker::end(), sys)
|
||||
|
|
||||
this = arr.(DataFlow::ArrayCreationNode).getAnElement()
|
||||
or
|
||||
this = arr.getAMethodCall(["push", "unshift"]).getAnArgument()
|
||||
)
|
||||
}
|
||||
|
||||
override string getSinkType() { result = "Shell argument" }
|
||||
|
||||
override SystemCommandExecution getCommandExecution() { result = sys }
|
||||
|
||||
override DataFlow::Node getAlertLocation() { result = this }
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer like: "'"+name.replace(/'/g,"'\\''")+"'"
|
||||
* Which sanitizes on Unix.
|
||||
|
||||
@@ -205,6 +205,30 @@ nodes
|
||||
| lib/lib.js:405:39:405:42 | name |
|
||||
| lib/lib.js:406:22:406:25 | name |
|
||||
| lib/lib.js:406:22:406:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name |
|
||||
| lib/lib.js:414:40:414:43 | name |
|
||||
| lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:428:28:428:51 | (name ? ... ' : '') |
|
||||
| lib/lib.js:428:28:428:57 | (name ? ... ) + '-' |
|
||||
| lib/lib.js:428:29:428:50 | name ? ... :' : '' |
|
||||
| lib/lib.js:428:36:428:39 | name |
|
||||
| lib/lib.js:428:36:428:45 | name + ':' |
|
||||
| lib/lib.js:431:23:431:26 | last |
|
||||
| lib/lib.js:436:19:436:22 | last |
|
||||
| lib/lib.js:436:19:436:22 | last |
|
||||
edges
|
||||
| lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
|
||||
| lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
|
||||
@@ -444,6 +468,43 @@ edges
|
||||
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
|
||||
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
|
||||
| lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:415:22:415:25 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:417:28:417:31 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:418:25:418:28 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:419:32:419:35 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:424:24:424:27 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:426:11:426:14 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:428:36:428:39 | name |
|
||||
| lib/lib.js:414:40:414:43 | name | lib/lib.js:428:36:428:39 | name |
|
||||
| lib/lib.js:428:28:428:51 | (name ? ... ' : '') | lib/lib.js:428:28:428:57 | (name ? ... ) + '-' |
|
||||
| lib/lib.js:428:28:428:57 | (name ? ... ) + '-' | lib/lib.js:431:23:431:26 | last |
|
||||
| lib/lib.js:428:29:428:50 | name ? ... :' : '' | lib/lib.js:428:28:428:51 | (name ? ... ' : '') |
|
||||
| lib/lib.js:428:36:428:39 | name | lib/lib.js:428:36:428:45 | name + ':' |
|
||||
| lib/lib.js:428:36:428:45 | name + ':' | lib/lib.js:428:29:428:50 | name ? ... :' : '' |
|
||||
| lib/lib.js:431:23:431:26 | last | lib/lib.js:436:19:436:22 | last |
|
||||
| lib/lib.js:431:23:431:26 | last | lib/lib.js:436:19:436:22 | last |
|
||||
#select
|
||||
| lib/lib2.js:4:10:4:25 | "rm -rf " + name | lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/lib2.js:4:2:4:26 | cp.exec ... + name) | shell command |
|
||||
| lib/lib2.js:8:10:8:25 | "rm -rf " + name | lib/lib2.js:7:32:7:35 | name | lib/lib2.js:8:22:8:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/lib2.js:8:2:8:26 | cp.exec ... + name) | shell command |
|
||||
@@ -502,3 +563,11 @@ edges
|
||||
| lib/lib.js:351:10:351:27 | "rm -rf " + unsafe | lib/lib.js:349:29:349:34 | unsafe | lib/lib.js:351:22:351:27 | unsafe | $@ based on library input is later used in $@. | lib/lib.js:351:10:351:27 | "rm -rf " + unsafe | String concatenation | lib/lib.js:351:2:351:28 | cp.exec ... unsafe) | shell command |
|
||||
| lib/lib.js:366:17:366:56 | "learn ... + model | lib/lib.js:360:20:360:23 | opts | lib/lib.js:366:28:366:42 | this.learn_args | $@ based on library input is later used in $@. | lib/lib.js:366:17:366:56 | "learn ... + model | String concatenation | lib/lib.js:367:3:367:18 | cp.exec(command) | shell command |
|
||||
| lib/lib.js:406:10:406:25 | "rm -rf " + name | lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name | $@ based on library input is later used in $@. | lib/lib.js:406:10:406:25 | "rm -rf " + name | String concatenation | lib/lib.js:406:2:406:26 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:415:10:415:25 | "rm -rf " + name | lib/lib.js:414:40:414:43 | name | lib/lib.js:415:22:415:25 | name | $@ based on library input is later used in $@. | lib/lib.js:415:10:415:25 | "rm -rf " + name | String concatenation | lib/lib.js:415:2:415:26 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:417:28:417:31 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:417:28:417:31 | name | $@ based on library input is later used in $@. | lib/lib.js:417:28:417:31 | name | Shell argument | lib/lib.js:417:2:417:66 | cp.exec ... => {}) | shell command |
|
||||
| lib/lib.js:418:25:418:28 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:418:25:418:28 | name | $@ based on library input is later used in $@. | lib/lib.js:418:25:418:28 | name | Shell argument | lib/lib.js:418:2:418:45 | cp.spaw ... true}) | shell command |
|
||||
| lib/lib.js:419:32:419:35 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:419:32:419:35 | name | $@ based on library input is later used in $@. | lib/lib.js:419:32:419:35 | name | Shell argument | lib/lib.js:419:2:419:52 | cp.exec ... true}) | shell command |
|
||||
| lib/lib.js:420:29:420:32 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name | $@ based on library input is later used in $@. | lib/lib.js:420:29:420:32 | name | Shell argument | lib/lib.js:420:2:420:49 | cp.spaw ... true}) | shell command |
|
||||
| lib/lib.js:424:24:424:27 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:424:24:424:27 | name | $@ based on library input is later used in $@. | lib/lib.js:424:24:424:27 | name | Shell argument | lib/lib.js:424:2:424:40 | spawn(" ... WN_OPT) | shell command |
|
||||
| lib/lib.js:426:11:426:14 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:426:11:426:14 | name | $@ based on library input is later used in $@. | lib/lib.js:426:11:426:14 | name | Shell argument | lib/lib.js:427:2:427:28 | spawn(" ... WN_OPT) | shell command |
|
||||
| lib/lib.js:436:19:436:22 | last | lib/lib.js:414:40:414:43 | name | lib/lib.js:436:19:436:22 | last | $@ based on library input is later used in $@. | lib/lib.js:436:19:436:22 | last | Shell argument | lib/lib.js:428:2:428:70 | spawn(" ... WN_OPT) | shell command |
|
||||
|
||||
@@ -53,6 +53,8 @@ syncCommand
|
||||
| command-line-parameter-command-injection.js:20:2:20:30 | cp.exec ... + arg0) |
|
||||
| command-line-parameter-command-injection.js:26:2:26:51 | cp.exec ... tion"`) |
|
||||
| command-line-parameter-command-injection.js:27:2:27:58 | cp.exec ... tion"`) |
|
||||
| lib/lib.js:419:2:419:52 | cp.exec ... true}) |
|
||||
| lib/lib.js:420:2:420:49 | cp.spaw ... true}) |
|
||||
| other.js:7:5:7:36 | require ... nc(cmd) |
|
||||
| other.js:9:5:9:35 | require ... nc(cmd) |
|
||||
| other.js:12:5:12:30 | require ... nc(cmd) |
|
||||
@@ -100,6 +102,13 @@ options
|
||||
| lib/lib.js:152:2:152:23 | cp.spaw ... gs, cb) | lib/lib.js:152:21:152:22 | cb |
|
||||
| lib/lib.js:159:2:159:23 | cp.spaw ... gs, cb) | lib/lib.js:159:21:159:22 | cb |
|
||||
| lib/lib.js:163:2:167:2 | cp.spaw ... t' }\\n\\t) | lib/lib.js:166:3:166:22 | { stdio: 'inherit' } |
|
||||
| lib/lib.js:417:2:417:66 | cp.exec ... => {}) | lib/lib.js:417:35:417:47 | {shell: true} |
|
||||
| lib/lib.js:418:2:418:45 | cp.spaw ... true}) | lib/lib.js:418:32:418:44 | {shell: true} |
|
||||
| lib/lib.js:419:2:419:52 | cp.exec ... true}) | lib/lib.js:419:39:419:51 | {shell: true} |
|
||||
| lib/lib.js:420:2:420:49 | cp.spaw ... true}) | lib/lib.js:420:36:420:48 | {shell: true} |
|
||||
| lib/lib.js:424:2:424:40 | spawn(" ... WN_OPT) | lib/lib.js:424:31:424:39 | SPAWN_OPT |
|
||||
| lib/lib.js:427:2:427:28 | spawn(" ... WN_OPT) | lib/lib.js:427:19:427:27 | SPAWN_OPT |
|
||||
| lib/lib.js:428:2:428:70 | spawn(" ... WN_OPT) | lib/lib.js:428:61:428:69 | SPAWN_OPT |
|
||||
| uselesscat.js:28:1:28:39 | execSyn ... 1000}) | uselesscat.js:28:28:28:38 | {uid: 1000} |
|
||||
| uselesscat.js:30:1:30:64 | exec('c ... t) { }) | uselesscat.js:30:26:30:38 | { cwd: './' } |
|
||||
| uselesscat.js:34:1:34:54 | execSyn ... utf8'}) | uselesscat.js:34:36:34:53 | {encoding: 'utf8'} |
|
||||
|
||||
@@ -408,3 +408,31 @@ module.exports.sanitizer3 = function (name) {
|
||||
var sanitized = yetAnohterSanitizer(name);
|
||||
cp.exec("rm -rf " + sanitized); // OK
|
||||
}
|
||||
|
||||
const cp = require("child_process");
|
||||
const spawn = cp.spawn;
|
||||
module.exports.shellOption = function (name) {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
|
||||
cp.execFile("rm", ["-rf", name], {shell: true}, (err, out) => {}); // NOT OK
|
||||
cp.spawn("rm", ["-rf", name], {shell: true}); // NOT OK
|
||||
cp.execFileSync("rm", ["-rf", name], {shell: true}); // NOT OK
|
||||
cp.spawnSync("rm", ["-rf", name], {shell: true}); // NOT OK
|
||||
|
||||
const SPAWN_OPT = {shell: true};
|
||||
|
||||
spawn("rm", ["first", name], SPAWN_OPT); // NOT OK
|
||||
var arr = [];
|
||||
arr.push(name); // NOT OK
|
||||
spawn("rm", arr, SPAWN_OPT);
|
||||
spawn("rm", build("node", (name ? name + ':' : '') + '-'), SPAWN_OPT); // This is bad, but the alert location is down in `build`.
|
||||
}
|
||||
|
||||
function build(first, last) {
|
||||
var arr = [];
|
||||
if (something() === 'gm')
|
||||
arr.push('convert');
|
||||
first && arr.push(first);
|
||||
last && arr.push(last); // NOT OK
|
||||
return arr;
|
||||
};
|
||||
Reference in New Issue
Block a user