Skip to content

Commit 8848042

Browse files
Copilotowen-mc
Copilot
and
owen-mc
authored
Recognize delegating command launchers
Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>
1 parent e29eac1 commit 8848042

4 files changed

Lines changed: 134 additions & 15 deletions

File tree

‎java/ql/lib/semmle/code/java/security/CommandArguments.qll‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,25 @@ import semmle.code.java.Collections
1010

1111
/**
1212
* Holds if `ex` is used safely as an argument to a command;
13-
* i.e. it's not in the first position and it's not a shell command.
13+
* i.e. it's not in the first position and the command does not interpret arguments as commands.
1414
*/
1515
predicate isSafeCommandArgument(Expr ex) {
1616
exists(ArrayInit ai, int i |
1717
ex = ai.getInit(i) and
1818
i > 0 and
19-
not isShell(ai.getInit(0))
19+
not isShell(ai.getInit(0)) and
20+
not isDelegatingLauncher(ai.getInit(0))
2021
)
2122
or
2223
exists(CommandArgumentList cal |
2324
not cal.isShell() and
25+
not isDelegatingLauncher(cal.getAFirstAdd().getArgument(0)) and
2426
ex = cal.getASubsequentAdd().getArgument(0)
2527
)
2628
or
2729
exists(CommandArgArrayImmutableFirst caa |
2830
not caa.isShell() and
31+
not isDelegatingLauncher(caa.getFirstElement()) and
2932
ex = caa.getAWrite(any(int i | i > 0))
3033
)
3134
}
@@ -52,6 +55,29 @@ private predicate isShell(Expr ex) {
5255
isShell(ex.(LocalVariableDeclExpr).getInit())
5356
}
5457

58+
/**
59+
* Holds if the given expression is the name of a launcher that delegates to a command supplied in
60+
* one of its arguments, rather than interpreting that command itself like a shell.
61+
*/
62+
private predicate isDelegatingLauncher(Expr ex) {
63+
exists(string cmd | cmd = ex.(StringLiteral).getValue() |
64+
cmd.regexpMatch("(.*/)?(chroot|env|nice|nohup|setsid|stdbuf|sudo|timeout|xargs)$")
65+
)
66+
or
67+
exists(SsaDefinition ssa, SsaExplicitWrite def |
68+
ex = ssa.getARead() and
69+
isDelegatingLauncher(def.getDefiningExpr())
70+
|
71+
ssa.getAnUltimateDefinition() = def
72+
or
73+
ssa.(SsaCapturedDefinition).getAnUltimateCapturedDefinition() = def
74+
)
75+
or
76+
isDelegatingLauncher(ex.(Assignment).getRhs())
77+
or
78+
isDelegatingLauncher(ex.(LocalVariableDeclExpr).getInit())
79+
}
80+
5581
/**
5682
* A type that could be a list of strings. Includes raw `List` types.
5783
*/

‎java/ql/test/query-tests/security/CWE-078/ExecRelative.expected‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,12 @@
11
| TaintedEnvironment.java:39:35:39:55 | new String[] | Command with a relative path 'ls' is executed. |
2-
| Test.java:50:46:50:49 | "ls" | Command with a relative path 'ls' is executed. |
2+
| Test.java:48:29:48:71 | new String[] | Command with a relative path 'cp' is executed. |
3+
| Test.java:53:24:53:63 | new String[] | Command with a relative path 'chroot' is executed. |
4+
| Test.java:54:24:54:47 | new String[] | Command with a relative path 'env' is executed. |
5+
| Test.java:55:24:55:48 | new String[] | Command with a relative path 'nice' is executed. |
6+
| Test.java:56:24:56:49 | new String[] | Command with a relative path 'nohup' is executed. |
7+
| Test.java:57:24:57:50 | new String[] | Command with a relative path 'setsid' is executed. |
8+
| Test.java:58:24:58:57 | new String[] | Command with a relative path 'stdbuf' is executed. |
9+
| Test.java:59:24:59:48 | new String[] | Command with a relative path 'sudo' is executed. |
10+
| Test.java:60:24:60:57 | new String[] | Command with a relative path 'timeout' is executed. |
11+
| Test.java:61:24:61:49 | new String[] | Command with a relative path 'xargs' is executed. |
12+
| Test.java:65:46:65:49 | "ls" | Command with a relative path 'ls' is executed. |

‎java/ql/test/query-tests/security/CWE-078/ExecTainted.expected‎

Lines changed: 79 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
#select
2-
| Test.java:7:44:7:69 | ... + ... | Test.java:57:27:57:39 | args : String[] | Test.java:7:25:7:70 | new ..[] { .. } | This command line depends on a $@. | Test.java:57:27:57:39 | args | user-provided value |
3-
| Test.java:10:29:10:74 | new String[] | Test.java:57:27:57:39 | args : String[] | Test.java:10:29:10:74 | new String[] | This command line depends on a $@. | Test.java:57:27:57:39 | args | user-provided value |
4-
| Test.java:18:29:18:31 | cmd | Test.java:57:27:57:39 | args : String[] | Test.java:18:29:18:31 | cmd | This command line depends on a $@. | Test.java:57:27:57:39 | args | user-provided value |
5-
| Test.java:24:29:24:32 | cmd1 | Test.java:57:27:57:39 | args : String[] | Test.java:24:29:24:32 | cmd1 | This command line depends on a $@. | Test.java:57:27:57:39 | args | user-provided value |
6-
| Test.java:29:44:29:64 | ... + ... | Test.java:57:27:57:39 | args : String[] | Test.java:29:25:29:65 | new ..[] { .. } | This command line depends on a $@. | Test.java:57:27:57:39 | args | user-provided value |
2+
| Test.java:7:44:7:69 | ... + ... | Test.java:72:27:72:39 | args : String[] | Test.java:7:25:7:70 | new ..[] { .. } | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
3+
| Test.java:10:29:10:74 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:10:29:10:74 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
4+
| Test.java:18:29:18:31 | cmd | Test.java:72:27:72:39 | args : String[] | Test.java:18:29:18:31 | cmd | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
5+
| Test.java:24:29:24:32 | cmd1 | Test.java:72:27:72:39 | args : String[] | Test.java:24:29:24:32 | cmd1 | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
6+
| Test.java:29:44:29:64 | ... + ... | Test.java:72:27:72:39 | args : String[] | Test.java:29:25:29:65 | new ..[] { .. } | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
7+
| Test.java:53:24:53:63 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:53:24:53:63 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
8+
| Test.java:54:24:54:47 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:54:24:54:47 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
9+
| Test.java:55:24:55:48 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:55:24:55:48 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
10+
| Test.java:56:24:56:49 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:56:24:56:49 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
11+
| Test.java:57:24:57:50 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:57:24:57:50 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
12+
| Test.java:58:24:58:57 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:58:24:58:57 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
13+
| Test.java:59:24:59:48 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:59:24:59:48 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
14+
| Test.java:60:24:60:57 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:60:24:60:57 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
15+
| Test.java:61:24:61:49 | new String[] | Test.java:72:27:72:39 | args : String[] | Test.java:61:24:61:49 | new String[] | This command line depends on a $@. | Test.java:72:27:72:39 | args | user-provided value |
716
edges
817
| Test.java:6:35:6:44 | arg : String | Test.java:7:44:7:69 | ... + ... : String | provenance | |
918
| Test.java:6:35:6:44 | arg : String | Test.java:10:61:10:73 | ... + ... : String | provenance | |
@@ -18,10 +27,39 @@ edges
1827
| Test.java:22:15:22:27 | ... + ... : String | Test.java:22:5:22:8 | cmd1 [post update] : String[] [[]] : String | provenance | |
1928
| Test.java:28:38:28:47 | arg : String | Test.java:29:44:29:64 | ... + ... : String | provenance | |
2029
| Test.java:29:44:29:64 | ... + ... : String | Test.java:29:25:29:65 | new ..[] { .. } | provenance | Sink:MaD:2 |
21-
| Test.java:57:27:57:39 | args : String[] | Test.java:60:20:60:22 | arg : String | provenance | |
22-
| Test.java:57:27:57:39 | args : String[] | Test.java:61:23:61:25 | arg : String | provenance | |
23-
| Test.java:60:20:60:22 | arg : String | Test.java:6:35:6:44 | arg : String | provenance | |
24-
| Test.java:61:23:61:25 | arg : String | Test.java:28:38:28:47 | arg : String | provenance | |
30+
| Test.java:52:42:52:51 | arg : String | Test.java:53:60:53:62 | arg : String | provenance | |
31+
| Test.java:52:42:52:51 | arg : String | Test.java:54:44:54:46 | arg : String | provenance | |
32+
| Test.java:52:42:52:51 | arg : String | Test.java:55:45:55:47 | arg : String | provenance | |
33+
| Test.java:52:42:52:51 | arg : String | Test.java:56:46:56:48 | arg : String | provenance | |
34+
| Test.java:52:42:52:51 | arg : String | Test.java:57:47:57:49 | arg : String | provenance | |
35+
| Test.java:52:42:52:51 | arg : String | Test.java:58:54:58:56 | arg : String | provenance | |
36+
| Test.java:52:42:52:51 | arg : String | Test.java:59:45:59:47 | arg : String | provenance | |
37+
| Test.java:52:42:52:51 | arg : String | Test.java:60:54:60:56 | arg : String | provenance | |
38+
| Test.java:52:42:52:51 | arg : String | Test.java:61:46:61:48 | arg : String | provenance | |
39+
| Test.java:53:24:53:63 | {...} : String[] [[]] : String | Test.java:53:24:53:63 | new String[] | provenance | Sink:MaD:2 |
40+
| Test.java:53:60:53:62 | arg : String | Test.java:53:24:53:63 | {...} : String[] [[]] : String | provenance | |
41+
| Test.java:54:24:54:47 | {...} : String[] [[]] : String | Test.java:54:24:54:47 | new String[] | provenance | Sink:MaD:2 |
42+
| Test.java:54:44:54:46 | arg : String | Test.java:54:24:54:47 | {...} : String[] [[]] : String | provenance | |
43+
| Test.java:55:24:55:48 | {...} : String[] [[]] : String | Test.java:55:24:55:48 | new String[] | provenance | Sink:MaD:2 |
44+
| Test.java:55:45:55:47 | arg : String | Test.java:55:24:55:48 | {...} : String[] [[]] : String | provenance | |
45+
| Test.java:56:24:56:49 | {...} : String[] [[]] : String | Test.java:56:24:56:49 | new String[] | provenance | Sink:MaD:2 |
46+
| Test.java:56:46:56:48 | arg : String | Test.java:56:24:56:49 | {...} : String[] [[]] : String | provenance | |
47+
| Test.java:57:24:57:50 | {...} : String[] [[]] : String | Test.java:57:24:57:50 | new String[] | provenance | Sink:MaD:2 |
48+
| Test.java:57:47:57:49 | arg : String | Test.java:57:24:57:50 | {...} : String[] [[]] : String | provenance | |
49+
| Test.java:58:24:58:57 | {...} : String[] [[]] : String | Test.java:58:24:58:57 | new String[] | provenance | Sink:MaD:2 |
50+
| Test.java:58:54:58:56 | arg : String | Test.java:58:24:58:57 | {...} : String[] [[]] : String | provenance | |
51+
| Test.java:59:24:59:48 | {...} : String[] [[]] : String | Test.java:59:24:59:48 | new String[] | provenance | Sink:MaD:2 |
52+
| Test.java:59:45:59:47 | arg : String | Test.java:59:24:59:48 | {...} : String[] [[]] : String | provenance | |
53+
| Test.java:60:24:60:57 | {...} : String[] [[]] : String | Test.java:60:24:60:57 | new String[] | provenance | Sink:MaD:2 |
54+
| Test.java:60:54:60:56 | arg : String | Test.java:60:24:60:57 | {...} : String[] [[]] : String | provenance | |
55+
| Test.java:61:24:61:49 | {...} : String[] [[]] : String | Test.java:61:24:61:49 | new String[] | provenance | Sink:MaD:2 |
56+
| Test.java:61:46:61:48 | arg : String | Test.java:61:24:61:49 | {...} : String[] [[]] : String | provenance | |
57+
| Test.java:72:27:72:39 | args : String[] | Test.java:75:20:75:22 | arg : String | provenance | |
58+
| Test.java:72:27:72:39 | args : String[] | Test.java:76:23:76:25 | arg : String | provenance | |
59+
| Test.java:72:27:72:39 | args : String[] | Test.java:77:27:77:29 | arg : String | provenance | |
60+
| Test.java:75:20:75:22 | arg : String | Test.java:6:35:6:44 | arg : String | provenance | |
61+
| Test.java:76:23:76:25 | arg : String | Test.java:28:38:28:47 | arg : String | provenance | |
62+
| Test.java:77:27:77:29 | arg : String | Test.java:52:42:52:51 | arg : String | provenance | |
2563
models
2664
| 1 | Sink: java.lang; ProcessBuilder; false; ProcessBuilder; (List); ; Argument[0]; command-injection; ai-manual |
2765
| 2 | Sink: java.lang; ProcessBuilder; false; ProcessBuilder; (String[]); ; Argument[0]; command-injection; ai-manual |
@@ -42,7 +80,36 @@ nodes
4280
| Test.java:28:38:28:47 | arg : String | semmle.label | arg : String |
4381
| Test.java:29:25:29:65 | new ..[] { .. } | semmle.label | new ..[] { .. } |
4482
| Test.java:29:44:29:64 | ... + ... : String | semmle.label | ... + ... : String |
45-
| Test.java:57:27:57:39 | args : String[] | semmle.label | args : String[] |
46-
| Test.java:60:20:60:22 | arg : String | semmle.label | arg : String |
47-
| Test.java:61:23:61:25 | arg : String | semmle.label | arg : String |
83+
| Test.java:52:42:52:51 | arg : String | semmle.label | arg : String |
84+
| Test.java:53:24:53:63 | new String[] | semmle.label | new String[] |
85+
| Test.java:53:24:53:63 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
86+
| Test.java:53:60:53:62 | arg : String | semmle.label | arg : String |
87+
| Test.java:54:24:54:47 | new String[] | semmle.label | new String[] |
88+
| Test.java:54:24:54:47 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
89+
| Test.java:54:44:54:46 | arg : String | semmle.label | arg : String |
90+
| Test.java:55:24:55:48 | new String[] | semmle.label | new String[] |
91+
| Test.java:55:24:55:48 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
92+
| Test.java:55:45:55:47 | arg : String | semmle.label | arg : String |
93+
| Test.java:56:24:56:49 | new String[] | semmle.label | new String[] |
94+
| Test.java:56:24:56:49 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
95+
| Test.java:56:46:56:48 | arg : String | semmle.label | arg : String |
96+
| Test.java:57:24:57:50 | new String[] | semmle.label | new String[] |
97+
| Test.java:57:24:57:50 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
98+
| Test.java:57:47:57:49 | arg : String | semmle.label | arg : String |
99+
| Test.java:58:24:58:57 | new String[] | semmle.label | new String[] |
100+
| Test.java:58:24:58:57 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
101+
| Test.java:58:54:58:56 | arg : String | semmle.label | arg : String |
102+
| Test.java:59:24:59:48 | new String[] | semmle.label | new String[] |
103+
| Test.java:59:24:59:48 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
104+
| Test.java:59:45:59:47 | arg : String | semmle.label | arg : String |
105+
| Test.java:60:24:60:57 | new String[] | semmle.label | new String[] |
106+
| Test.java:60:24:60:57 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
107+
| Test.java:60:54:60:56 | arg : String | semmle.label | arg : String |
108+
| Test.java:61:24:61:49 | new String[] | semmle.label | new String[] |
109+
| Test.java:61:24:61:49 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
110+
| Test.java:61:46:61:48 | arg : String | semmle.label | arg : String |
111+
| Test.java:72:27:72:39 | args : String[] | semmle.label | args : String[] |
112+
| Test.java:75:20:75:22 | arg : String | semmle.label | arg : String |
113+
| Test.java:76:23:76:25 | arg : String | semmle.label | arg : String |
114+
| Test.java:77:27:77:29 | arg : String | semmle.label | arg : String |
48115
subpaths

‎java/ql/test/query-tests/security/CWE-078/Test.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,21 @@ public static void nonShellCommand(String arg) throws java.io.IOException {
4444

4545
pb = new ProcessBuilder(cmd1);
4646
pb.start();
47+
48+
pb = new ProcessBuilder(new String[]{"cp", arg, "/tmp/destination"}); // $ Alert[java/relative-path-command]
49+
pb.start();
50+
}
51+
52+
public static void delegatingLaunchers(String arg) throws java.io.IOException {
53+
new ProcessBuilder(new String[]{"chroot", "/tmp/root", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
54+
new ProcessBuilder(new String[]{"env", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
55+
new ProcessBuilder(new String[]{"nice", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
56+
new ProcessBuilder(new String[]{"nohup", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
57+
new ProcessBuilder(new String[]{"setsid", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
58+
new ProcessBuilder(new String[]{"stdbuf", "-oL", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
59+
new ProcessBuilder(new String[]{"sudo", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
60+
new ProcessBuilder(new String[]{"timeout", "10", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
61+
new ProcessBuilder(new String[]{"xargs", arg}).start(); // $ Alert[java/command-line-injection] Alert[java/relative-path-command]
4762
}
4863

4964
public static void relativeCommand() throws java.io.IOException {
@@ -59,6 +74,7 @@ public static void main(String[] args) throws java.io.IOException { // $ Source[
5974

6075
shellCommand(arg);
6176
nonShellCommand(arg);
77+
delegatingLaunchers(arg);
6278
relativeCommand();
6379
}
6480
}

0 commit comments

Comments
 (0)