Merge pull request #11082 from erik-krogh/shellArr

JS: treat arrays that gets executed with shell:true as a sink for `js/shell-command-constructed-from-input`
This commit is contained in:
Erik Krogh Kristensen
2022-11-22 13:03:50 +01:00
committed by GitHub
3 changed files with 45 additions and 8 deletions

View File

@@ -156,14 +156,9 @@ module UnsafeShellCommandConstruction {
}
/**
* Gets a node that ends up in an array that is ultimately executed as a shell script by `sys`.
* Holds if the arguments array given to `sys` is joined as a string because `shell` is set to true.
*/
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.
predicate executesArrayAsShell(SystemCommandExecution sys) {
sys.getOptionsArg()
.getALocalSource()
.getAPropertyWrite("shell")
@@ -171,6 +166,17 @@ module UnsafeShellCommandConstruction {
.asExpr()
.(BooleanLiteral)
.getValue() = "true"
}
/**
* 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
executesArrayAsShell(sys)
or
exists(DataFlow::TypeBackTracker t2 |
result = endsInShellExecutedArray(t2, sys).backtrack(t2, t)
@@ -193,6 +199,10 @@ module UnsafeShellCommandConstruction {
or
this = arr.getAMethodCall(["push", "unshift"]).getAnArgument()
)
or
this = sys.getArgumentList() and
not this instanceof DataFlow::ArrayCreationNode and
executesArrayAsShell(sys)
}
override string getSinkType() { result = "shell argument" }

View File

@@ -227,8 +227,14 @@ nodes
| 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:425:6:425:13 | arr |
| lib/lib.js:425:12:425:13 | [] |
| lib/lib.js:426:11:426:14 | name |
| lib/lib.js:426:11:426:14 | name |
| lib/lib.js:427:14:427:16 | arr |
| lib/lib.js:427:14:427:16 | arr |
| lib/lib.js:428:14:428:58 | build(" ... + '-') |
| lib/lib.js:428:14:428:58 | build(" ... + '-') |
| lib/lib.js:428:28:428:51 | (name ? ... ' : '') |
| lib/lib.js:428:28:428:57 | (name ? ... ) + '-' |
| lib/lib.js:428:29:428:50 | name ? ... :' : '' |
@@ -306,6 +312,10 @@ nodes
| lib/subLib/index.js:7:32:7:35 | name |
| lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:13:44:13:46 | arr |
| lib/subLib/index.js:13:44:13:46 | arr |
| lib/subLib/index.js:14:22:14:24 | arr |
| lib/subLib/index.js:14:22:14:24 | arr |
edges
| lib/isImported.js:5:49:5:52 | name | lib/isImported.js:6:22:6:25 | name |
| lib/isImported.js:5:49:5:52 | name | lib/isImported.js:6:22:6:25 | name |
@@ -584,7 +594,13 @@ edges
| 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:425:6:425:13 | arr | lib/lib.js:427:14:427:16 | arr |
| lib/lib.js:425:6:425:13 | arr | lib/lib.js:427:14:427:16 | arr |
| lib/lib.js:425:12:425:13 | [] | lib/lib.js:425:6:425:13 | arr |
| lib/lib.js:426:11:426:14 | name | lib/lib.js:425:12:425:13 | [] |
| 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:428:14:428:58 | build(" ... + '-') |
| lib/lib.js:428:28:428:57 | (name ? ... ) + '-' | lib/lib.js:428:14:428:58 | build(" ... + '-') |
| 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 + ':' |
@@ -672,6 +688,10 @@ edges
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:13:44:13:46 | arr | lib/subLib/index.js:14:22:14:24 | arr |
| lib/subLib/index.js:13:44:13:46 | arr | lib/subLib/index.js:14:22:14:24 | arr |
| lib/subLib/index.js:13:44:13:46 | arr | lib/subLib/index.js:14:22:14:24 | arr |
| lib/subLib/index.js:13:44:13:46 | arr | lib/subLib/index.js:14:22:14:24 | arr |
#select
| lib/isImported.js:6:10:6:25 | "rm -rf " + name | lib/isImported.js:5:49:5:52 | name | lib/isImported.js:6:22:6:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/isImported.js:5:49:5:52 | name | library input | lib/isImported.js:6:2:6:26 | cp.exec ... + name) | shell command |
| 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 | This string concatenation which depends on $@ is later used in a $@. | lib/lib2.js:3:28:3:31 | name | library input | lib/lib2.js:4:2:4:26 | cp.exec ... + name) | shell command |
@@ -739,6 +759,8 @@ edges
| lib/lib.js:420:29:420:32 | name | lib/lib.js:414:40:414:43 | name | lib/lib.js:420:29:420:32 | name | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | 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 | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | 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 | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | lib/lib.js:427:2:427:28 | spawn(" ... WN_OPT) | shell command |
| lib/lib.js:427:14:427:16 | arr | lib/lib.js:414:40:414:43 | name | lib/lib.js:427:14:427:16 | arr | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | lib/lib.js:427:2:427:28 | spawn(" ... WN_OPT) | shell command |
| lib/lib.js:428:14:428:58 | build(" ... + '-') | lib/lib.js:414:40:414:43 | name | lib/lib.js:428:14:428:58 | build(" ... + '-') | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | lib/lib.js:428:2:428:70 | 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 | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:414:40:414:43 | name | library input | lib/lib.js:428:2:428:70 | spawn(" ... WN_OPT) | shell command |
| lib/lib.js:442:12:442:27 | "rm -rf " + name | lib/lib.js:441:39:441:42 | name | lib/lib.js:442:24:442:27 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:441:39:441:42 | name | library input | lib/lib.js:442:2:442:28 | asyncEx ... + name) | shell command |
| lib/lib.js:447:13:447:28 | "rm -rf " + name | lib/lib.js:446:20:446:23 | name | lib/lib.js:447:25:447:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:446:20:446:23 | name | library input | lib/lib.js:447:3:447:29 | asyncEx ... + name) | shell command |
@@ -760,3 +782,4 @@ edges
| lib/subLib/amdSub.js:4:10:4:25 | "rm -rf " + name | lib/subLib/amdSub.js:3:28:3:31 | name | lib/subLib/amdSub.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib/amdSub.js:3:28:3:31 | name | library input | lib/subLib/amdSub.js:4:2:4:26 | cp.exec ... + name) | shell command |
| lib/subLib/index.js:4:10:4:25 | "rm -rf " + name | lib/subLib/index.js:3:28:3:31 | name | lib/subLib/index.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib/index.js:3:28:3:31 | name | library input | lib/subLib/index.js:4:2:4:26 | cp.exec ... + name) | shell command |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib/index.js:7:32:7:35 | name | library input | lib/subLib/index.js:8:2:8:26 | cp.exec ... + name) | shell command |
| lib/subLib/index.js:14:22:14:24 | arr | lib/subLib/index.js:13:44:13:46 | arr | lib/subLib/index.js:14:22:14:24 | arr | This shell argument which depends on $@ is later used in a $@. | lib/subLib/index.js:13:44:13:46 | arr | library input | lib/subLib/index.js:14:5:14:40 | cp.spaw ... true}) | shell command |

View File

@@ -8,4 +8,8 @@ module.exports.foo = function (name) {
cp.exec("rm -rf " + name); // NOT OK - this is being called explicitly from child_process-test.js
};
module.exports.amd = require("./amd.js");
module.exports.amd = require("./amd.js");
module.exports.arrToShell = function (cmd, arr) {
cp.spawn("echo", arr, {shell: true}); // NOT OK
}