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
2022-11-02 14:06:57 +01:00
parent bc5b7455cf
commit 40032f295a
3 changed files with 41 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

@@ -223,8 +223,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 ? ... :' : '' |
@@ -302,6 +308,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 |
@@ -575,7 +585,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 + ':' |
@@ -663,6 +679,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 |
@@ -729,6 +749,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 |
@@ -750,3 +772,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

@@ -11,5 +11,5 @@ module.exports.foo = function (name) {
module.exports.amd = require("./amd.js");
module.exports.arrToShell = function (cmd, arr) {
cp.spawn("echo", arr, {shell: true}); // NOT OK - but not flagged [INCONSISTENCY]
cp.spawn("echo", arr, {shell: true}); // NOT OK
}