Skip to content

Commit e8e2f7b

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #2240 from max-schaefer/js/indirect-command-argument-data-flow
Approved by esbena
2 parents ea23c2d + 8aae1f4 commit e8e2f7b

File tree

4 files changed

+26
-223
lines changed

4 files changed

+26
-223
lines changed

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

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import javascript
1010
* That is, either `shell` is a Unix shell (`sh` or similar) and
1111
* `arg` is `"-c"`, or `shell` is `cmd.exe` and `arg` is `"/c"`.
1212
*/
13-
private predicate shellCmd(ConstantString shell, string arg) {
13+
private predicate shellCmd(Expr shell, string arg) {
1414
exists(string s | s = shell.getStringValue() |
1515
(s = "sh" or s = "bash" or s = "/bin/sh" or s = "/bin/bash") and
1616
arg = "-c"
@@ -23,25 +23,29 @@ private predicate shellCmd(ConstantString shell, string arg) {
2323
}
2424

2525
/**
26-
* Data flow configuration for tracking string literals that look like they
27-
* may refer to an operating-system shell, and array literals that may end up being
28-
* interpreted as argument lists for system commands.
26+
* Gets a data-flow node whose value ends up being interpreted as the command argument in `sys`
27+
* after a flow summarized by `t`.
2928
*/
30-
private class ArgumentListTracking extends DataFlow::Configuration {
31-
ArgumentListTracking() { this = "ArgumentListTracking" }
32-
33-
override predicate isSource(DataFlow::Node nd) {
34-
nd instanceof DataFlow::ArrayCreationNode
35-
or
36-
exists(ConstantString shell | shellCmd(shell, _) | nd = DataFlow::valueNode(shell))
37-
}
29+
private DataFlow::Node commandArgument(SystemCommandExecution sys, DataFlow::TypeBackTracker t) {
30+
t.start() and
31+
result = sys.getACommandArgument()
32+
or
33+
exists(DataFlow::TypeBackTracker t2 |
34+
t = t2.smallstep(result, commandArgument(sys, t2))
35+
)
36+
}
3837

39-
override predicate isSink(DataFlow::Node nd) {
40-
exists(SystemCommandExecution sys |
41-
nd = sys.getACommandArgument() or
42-
nd = sys.getArgumentList()
43-
)
44-
}
38+
/**
39+
* Gets a data-flow node whose value ends up being interpreted as the argument list in `sys`
40+
* after a flow summarized by `t`.
41+
*/
42+
private DataFlow::SourceNode argumentList(SystemCommandExecution sys, DataFlow::TypeBackTracker t) {
43+
t.start() and
44+
result = sys.getArgumentList().getALocalSource()
45+
or
46+
exists(DataFlow::TypeBackTracker t2 |
47+
result = argumentList(sys, t2).backtrack(t2, t)
48+
)
4549
}
4650

4751
/**
@@ -60,11 +64,11 @@ private class ArgumentListTracking extends DataFlow::Configuration {
6064
*/
6165
predicate isIndirectCommandArgument(DataFlow::Node source, SystemCommandExecution sys) {
6266
exists(
63-
ArgumentListTracking cfg, DataFlow::ArrayCreationNode args, ConstantString shell, string dashC
67+
DataFlow::ArrayCreationNode args, DataFlow::Node shell, string dashC
6468
|
65-
shellCmd(shell, dashC) and
66-
cfg.hasFlow(DataFlow::valueNode(shell), sys.getACommandArgument()) and
67-
cfg.hasFlow(args, sys.getArgumentList()) and
69+
shellCmd(shell.asExpr(), dashC) and
70+
shell = commandArgument(sys, DataFlow::TypeBackTracker::end()) and
71+
args = argumentList(sys, DataFlow::TypeBackTracker::end()) and
6872
args.getAPropertyWrite().getRhs().mayHaveStringValue(dashC) and
6973
source = args.getAPropertyWrite().getRhs()
7074
)

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

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -22,48 +22,12 @@ nodes
2222
| child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" |
2323
| child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" |
2424
| child_process-test.js:25:21:25:23 | cmd |
25-
| child_process-test.js:36:7:36:20 | sh |
26-
| child_process-test.js:36:12:36:20 | 'cmd.exe' |
27-
| child_process-test.js:36:12:36:20 | 'cmd.exe' |
28-
| child_process-test.js:38:7:38:20 | sh |
29-
| child_process-test.js:38:12:38:20 | '/bin/sh' |
30-
| child_process-test.js:38:12:38:20 | '/bin/sh' |
31-
| child_process-test.js:39:14:39:15 | sh |
32-
| child_process-test.js:39:14:39:15 | sh |
33-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
34-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
35-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
3625
| child_process-test.js:39:26:39:28 | cmd |
3726
| child_process-test.js:39:26:39:28 | cmd |
38-
| child_process-test.js:41:9:41:17 | args |
39-
| child_process-test.js:41:16:41:17 | [] |
40-
| child_process-test.js:41:16:41:17 | [] |
4127
| child_process-test.js:43:15:43:17 | cmd |
4228
| child_process-test.js:43:15:43:17 | cmd |
43-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
44-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
45-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
46-
| child_process-test.js:44:30:44:33 | args |
47-
| child_process-test.js:44:30:44:33 | args |
48-
| child_process-test.js:46:9:46:12 | "sh" |
49-
| child_process-test.js:46:9:46:12 | "sh" |
50-
| child_process-test.js:46:15:46:18 | args |
51-
| child_process-test.js:48:9:48:17 | args |
52-
| child_process-test.js:48:16:48:17 | [] |
53-
| child_process-test.js:48:16:48:17 | [] |
5429
| child_process-test.js:50:15:50:17 | cmd |
5530
| child_process-test.js:50:15:50:17 | cmd |
56-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
57-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
58-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
59-
| child_process-test.js:51:35:51:38 | args |
60-
| child_process-test.js:51:35:51:38 | args |
61-
| child_process-test.js:55:14:55:16 | cmd |
62-
| child_process-test.js:55:19:55:22 | args |
63-
| child_process-test.js:56:12:56:14 | cmd |
64-
| child_process-test.js:56:12:56:14 | cmd |
65-
| child_process-test.js:56:17:56:20 | args |
66-
| child_process-test.js:56:17:56:20 | args |
6731
| execSeries.js:3:20:3:22 | arr |
6832
| execSeries.js:6:14:6:16 | arr |
6933
| execSeries.js:6:14:6:21 | arr[i++] |
@@ -114,9 +78,6 @@ nodes
11478
| third-party-command-injection.js:5:20:5:26 | command |
11579
| third-party-command-injection.js:6:21:6:27 | command |
11680
| third-party-command-injection.js:6:21:6:27 | command |
117-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
118-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
119-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
12081
edges
12182
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
12283
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:17:13:17:15 | cmd |
@@ -146,33 +107,6 @@ edges
146107
| child_process-test.js:6:25:6:31 | req.url | child_process-test.js:6:15:6:38 | url.par ... , true) |
147108
| child_process-test.js:25:21:25:23 | cmd | child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" |
148109
| child_process-test.js:25:21:25:23 | cmd | child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" |
149-
| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh |
150-
| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh |
151-
| child_process-test.js:36:12:36:20 | 'cmd.exe' | child_process-test.js:36:7:36:20 | sh |
152-
| child_process-test.js:36:12:36:20 | 'cmd.exe' | child_process-test.js:36:7:36:20 | sh |
153-
| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:14:39:15 | sh |
154-
| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:14:39:15 | sh |
155-
| child_process-test.js:38:12:38:20 | '/bin/sh' | child_process-test.js:38:7:38:20 | sh |
156-
| child_process-test.js:38:12:38:20 | '/bin/sh' | child_process-test.js:38:7:38:20 | sh |
157-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] | child_process-test.js:39:18:39:30 | [ flag, cmd ] |
158-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args |
159-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args |
160-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:46:15:46:18 | args |
161-
| child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args |
162-
| child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args |
163-
| child_process-test.js:44:17:44:27 | "/bin/bash" | child_process-test.js:44:17:44:27 | "/bin/bash" |
164-
| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd |
165-
| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd |
166-
| child_process-test.js:46:15:46:18 | args | child_process-test.js:55:19:55:22 | args |
167-
| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args |
168-
| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args |
169-
| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args |
170-
| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args |
171-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" | child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
172-
| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd |
173-
| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd |
174-
| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args |
175-
| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args |
176110
| execSeries.js:3:20:3:22 | arr | execSeries.js:6:14:6:16 | arr |
177111
| execSeries.js:6:14:6:16 | arr | execSeries.js:6:14:6:21 | arr[i++] |
178112
| execSeries.js:6:14:6:21 | arr[i++] | execSeries.js:14:24:14:30 | command |
@@ -222,7 +156,6 @@ edges
222156
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command |
223157
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command |
224158
| third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command |
225-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] | tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
226159
#select
227160
| child_process-test.js:17:13:17:15 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:17:13:17:15 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
228161
| child_process-test.js:18:17:18:19 | cmd | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:18:17:18:19 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |

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

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,4 @@
11
nodes
2-
| child_process-test.js:36:7:36:20 | sh |
3-
| child_process-test.js:36:12:36:20 | 'cmd.exe' |
4-
| child_process-test.js:36:12:36:20 | 'cmd.exe' |
5-
| child_process-test.js:38:7:38:20 | sh |
6-
| child_process-test.js:38:12:38:20 | '/bin/sh' |
7-
| child_process-test.js:38:12:38:20 | '/bin/sh' |
8-
| child_process-test.js:39:14:39:15 | sh |
9-
| child_process-test.js:39:14:39:15 | sh |
10-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
11-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
12-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] |
13-
| child_process-test.js:41:9:41:17 | args |
14-
| child_process-test.js:41:16:41:17 | [] |
15-
| child_process-test.js:41:16:41:17 | [] |
16-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
17-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
18-
| child_process-test.js:44:17:44:27 | "/bin/bash" |
19-
| child_process-test.js:44:30:44:33 | args |
20-
| child_process-test.js:44:30:44:33 | args |
21-
| child_process-test.js:46:9:46:12 | "sh" |
22-
| child_process-test.js:46:9:46:12 | "sh" |
23-
| child_process-test.js:46:15:46:18 | args |
24-
| child_process-test.js:48:9:48:17 | args |
25-
| child_process-test.js:48:16:48:17 | [] |
26-
| child_process-test.js:48:16:48:17 | [] |
27-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
28-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
29-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
30-
| child_process-test.js:51:35:51:38 | args |
31-
| child_process-test.js:51:35:51:38 | args |
32-
| child_process-test.js:55:14:55:16 | cmd |
33-
| child_process-test.js:55:19:55:22 | args |
34-
| child_process-test.js:56:12:56:14 | cmd |
35-
| child_process-test.js:56:12:56:14 | cmd |
36-
| child_process-test.js:56:17:56:20 | args |
37-
| child_process-test.js:56:17:56:20 | args |
382
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
393
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
404
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
@@ -84,37 +48,7 @@ nodes
8448
| command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
8549
| command-line-parameter-command-injection.js:27:32:27:35 | args |
8650
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') |
87-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
88-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
89-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
9051
edges
91-
| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh |
92-
| child_process-test.js:36:7:36:20 | sh | child_process-test.js:39:14:39:15 | sh |
93-
| child_process-test.js:36:12:36:20 | 'cmd.exe' | child_process-test.js:36:7:36:20 | sh |
94-
| child_process-test.js:36:12:36:20 | 'cmd.exe' | child_process-test.js:36:7:36:20 | sh |
95-
| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:14:39:15 | sh |
96-
| child_process-test.js:38:7:38:20 | sh | child_process-test.js:39:14:39:15 | sh |
97-
| child_process-test.js:38:12:38:20 | '/bin/sh' | child_process-test.js:38:7:38:20 | sh |
98-
| child_process-test.js:38:12:38:20 | '/bin/sh' | child_process-test.js:38:7:38:20 | sh |
99-
| child_process-test.js:39:18:39:30 | [ flag, cmd ] | child_process-test.js:39:18:39:30 | [ flag, cmd ] |
100-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args |
101-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:44:30:44:33 | args |
102-
| child_process-test.js:41:9:41:17 | args | child_process-test.js:46:15:46:18 | args |
103-
| child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args |
104-
| child_process-test.js:41:16:41:17 | [] | child_process-test.js:41:9:41:17 | args |
105-
| child_process-test.js:44:17:44:27 | "/bin/bash" | child_process-test.js:44:17:44:27 | "/bin/bash" |
106-
| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd |
107-
| child_process-test.js:46:9:46:12 | "sh" | child_process-test.js:55:14:55:16 | cmd |
108-
| child_process-test.js:46:15:46:18 | args | child_process-test.js:55:19:55:22 | args |
109-
| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args |
110-
| child_process-test.js:48:9:48:17 | args | child_process-test.js:51:35:51:38 | args |
111-
| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args |
112-
| child_process-test.js:48:16:48:17 | [] | child_process-test.js:48:9:48:17 | args |
113-
| child_process-test.js:51:17:51:32 | `/bin` + "/bash" | child_process-test.js:51:17:51:32 | `/bin` + "/bash" |
114-
| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd |
115-
| child_process-test.js:55:14:55:16 | cmd | child_process-test.js:56:12:56:14 | cmd |
116-
| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args |
117-
| child_process-test.js:55:19:55:22 | args | child_process-test.js:56:17:56:20 | args |
11852
| command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
11953
| 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] |
12054
| 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] |
@@ -159,7 +93,6 @@ edges
15993
| command-line-parameter-command-injection.js:27:32:27:35 | args | command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') |
16094
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
16195
| command-line-parameter-command-injection.js:27:32:27:45 | args.join(' ') | command-line-parameter-command-injection.js:27:14:27:57 | `node $ ... ption"` |
162-
| tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] | tst_shell-command-injection-from-environment.js:4:25:4:61 | ['-rf', ... temp")] |
16396
#select
16497
| 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 |
16598
| 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 |

0 commit comments

Comments
 (0)