Skip to content

Commit 33eaeb9

Browse files
atorralbaatorralba
authored
Merge pull request github#13484 from aegilops/java/experimental/command-injection
Java: Experimental version of Java Command Injection query
2 parents 51c8331 + 5db569d commit 33eaeb9

9 files changed

+339
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class Test {
2+
public static void main(String[] args) {
3+
String script = System.getenv("SCRIPTNAME");
4+
if (script != null) {
5+
// BAD: The script to be executed by /bin/sh is controlled by the user.
6+
Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
7+
}
8+
}
9+
}

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qhelp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Code that passes remote user input to an arugment of a call of <code>Runtime.exec</code> that
7+
executes a scripting executable will allow the user to execute malicious code.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>If possible, use hard-coded string literals to specify the command or script to run,
13+
or library to load. Instead of passing the user input directly to the
14+
process or library function, examine the user input and then choose
15+
among hard-coded string literals.</p>
16+
17+
<p>If the applicable libraries or commands cannot be determined at
18+
compile time, then add code to verify that the user input string is
19+
safe before using it.</p>
20+
21+
</recommendation>
22+
<example>
23+
24+
<p>The following example shows code that takes a shell script that can be changed
25+
maliciously by a user, and passes it straight to the array going into <code>Runtime.exec</code>
26+
without examining it first.</p>
27+
28+
<sample src="CommandInjectionRuntimeExec.java" />
29+
30+
</example>
31+
<references>
32+
33+
<li>
34+
OWASP:
35+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
36+
</li>
37+
<li>SEI CERT Oracle Coding Standard for Java:
38+
<a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method">IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method</a>.</li>
39+
40+
</references>
41+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.ql

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* @name Command Injection into Runtime.exec() with dangerous command
3+
* @description High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 6.1
7+
* @precision high
8+
* @id java/command-line-injection-extra
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-078
12+
*/
13+
14+
import CommandInjectionRuntimeExec
15+
import ExecUserFlow::PathGraph
16+
17+
class RemoteSource extends Source instanceof RemoteFlowSource { }
18+
19+
from
20+
ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd,
21+
DataFlow::Node sinkCmd
22+
where callIsTaintedByUserInputAndDangerousCommand(source, sink, sourceCmd, sinkCmd)
23+
select sink, source, sink,
24+
"Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
25+
sourceCmd, sourceCmd.toString(), source.getNode(), source.toString()

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExec.qll

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import java
2+
import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
3+
import semmle.code.java.dataflow.DataFlow
4+
import semmle.code.java.dataflow.FlowSources
5+
6+
module ExecCmdFlowConfig implements DataFlow::ConfigSig {
7+
predicate isSource(DataFlow::Node source) {
8+
source.asExpr().(CompileTimeConstantExpr).getStringValue() instanceof UnSafeExecutable
9+
}
10+
11+
predicate isSink(DataFlow::Node sink) {
12+
exists(MethodAccess call |
13+
call.getMethod() instanceof RuntimeExecMethod and
14+
sink.asExpr() = call.getArgument(0) and
15+
sink.asExpr().getType() instanceof Array
16+
)
17+
}
18+
19+
predicate isBarrier(DataFlow::Node node) {
20+
node instanceof AssignToNonZeroIndex or
21+
node instanceof ArrayInitAtNonZeroIndex or
22+
node instanceof StreamConcatAtNonZeroIndex or
23+
node.getType() instanceof PrimitiveType or
24+
node.getType() instanceof BoxedType
25+
}
26+
}
27+
28+
/** Tracks flow of unvalidated user input that is used in Runtime.Exec */
29+
module ExecCmdFlow = TaintTracking::Global<ExecCmdFlowConfig>;
30+
31+
abstract class Source extends DataFlow::Node { }
32+
33+
module ExecUserFlowConfig implements DataFlow::ConfigSig {
34+
predicate isSource(DataFlow::Node source) { source instanceof Source }
35+
36+
predicate isSink(DataFlow::Node sink) {
37+
exists(MethodAccess call |
38+
call.getMethod() instanceof RuntimeExecMethod and
39+
sink.asExpr() = call.getArgument(_) and
40+
sink.asExpr().getType() instanceof Array
41+
)
42+
}
43+
44+
predicate isBarrier(DataFlow::Node node) {
45+
node.getType() instanceof PrimitiveType or
46+
node.getType() instanceof BoxedType
47+
}
48+
}
49+
50+
/** Tracks flow of unvalidated user input that is used in Runtime.Exec */
51+
module ExecUserFlow = TaintTracking::Global<ExecUserFlowConfig>;
52+
53+
// array[3] = node
54+
class AssignToNonZeroIndex extends DataFlow::Node {
55+
AssignToNonZeroIndex() {
56+
exists(AssignExpr assign, ArrayAccess access |
57+
assign.getDest() = access and
58+
access.getIndexExpr().(IntegerLiteral).getValue().toInt() != 0 and
59+
assign.getSource() = this.asExpr()
60+
)
61+
}
62+
}
63+
64+
// String[] array = {"a", "b, "c"};
65+
class ArrayInitAtNonZeroIndex extends DataFlow::Node {
66+
ArrayInitAtNonZeroIndex() {
67+
exists(ArrayInit init, int index |
68+
init.getInit(index) = this.asExpr() and
69+
index != 0
70+
)
71+
}
72+
}
73+
74+
// Stream.concat(Arrays.stream(array_1), Arrays.stream(array_2))
75+
class StreamConcatAtNonZeroIndex extends DataFlow::Node {
76+
StreamConcatAtNonZeroIndex() {
77+
exists(MethodAccess call, int index |
78+
call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat" and
79+
call.getArgument(index) = this.asExpr() and
80+
index != 0
81+
)
82+
}
83+
}
84+
85+
// list of executables that execute their arguments
86+
// TODO: extend with data extensions
87+
class UnSafeExecutable extends string {
88+
bindingset[this]
89+
UnSafeExecutable() {
90+
this.regexpMatch("^(|.*/)([a-z]*sh|javac?|python.*|perl|[Pp]ower[Ss]hell|php|node|deno|bun|ruby|osascript|cmd|Rscript|groovy)(\\.exe)?$") and
91+
not this = "netsh.exe"
92+
}
93+
}
94+
95+
predicate callIsTaintedByUserInputAndDangerousCommand(
96+
ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd,
97+
DataFlow::Node sinkCmd
98+
) {
99+
exists(MethodAccess call |
100+
call.getMethod() instanceof RuntimeExecMethod and
101+
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
102+
ExecCmdFlow::flow(sourceCmd, sinkCmd) and
103+
sinkCmd.asExpr() = call.getArgument(0) and
104+
// it is tainted by untrusted user input
105+
ExecUserFlow::flowPath(source, sink) and
106+
sink.getNode().asExpr() = call.getArgument(0)
107+
)
108+
}

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.qhelp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Code that passes local user input to an arugment of a call of <code>Runtime.exec</code> that
7+
executes a scripting executable will allow the user to execute malicious code.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>If possible, use hard-coded string literals to specify the command or script to run,
13+
or library to load. Instead of passing the user input directly to the
14+
process or library function, examine the user input and then choose
15+
among hard-coded string literals.</p>
16+
17+
<p>If the applicable libraries or commands cannot be determined at
18+
compile time, then add code to verify that the user input string is
19+
safe before using it.</p>
20+
21+
</recommendation>
22+
<example>
23+
24+
<p>The following example shows code that takes a shell script that can be changed
25+
maliciously by a user, and passes it straight to the array going into <code>Runtime.exec</code>
26+
without examining it first.</p>
27+
28+
<sample src="CommandInjectionRuntimeExec.java" />
29+
30+
</example>
31+
<references>
32+
33+
<li>
34+
OWASP:
35+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
36+
</li>
37+
<li>SEI CERT Oracle Coding Standard for Java:
38+
<a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method">IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method</a>.</li>
39+
40+
</references>
41+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @name Command Injection into Runtime.exec() with dangerous command
3+
* @description High sensitvity and precision version of java/command-line-injection, designed to find more cases of command injection in rare cases that the default query does not find
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 6.1
7+
* @precision high
8+
* @id java/command-line-injection-extra-local
9+
* @tags security
10+
* experimental
11+
* local
12+
* external/cwe/cwe-078
13+
*/
14+
15+
import CommandInjectionRuntimeExec
16+
import ExecUserFlow::PathGraph
17+
18+
class LocalSource extends Source instanceof LocalUserInput { }
19+
20+
from
21+
ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd,
22+
DataFlow::Node sinkCmd
23+
where callIsTaintedByUserInputAndDangerousCommand(source, sink, sourceCmd, sinkCmd)
24+
select sink, source, sink,
25+
"Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
26+
sourceCmd, sourceCmd.toString(), source.getNode(), source.toString()

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
edges
2+
| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:22:67:22:72 | script : String |
3+
| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:25:66:25:71 | script : String |
4+
| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:31:36:31:41 | script : String |
5+
| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:38:52:38:57 | script : String |
6+
| RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String | RuntimeExecTest.java:22:43:22:73 | new String[] |
7+
| RuntimeExecTest.java:22:67:22:72 | script : String | RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String |
8+
| RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String | RuntimeExecTest.java:26:43:26:55 | commandArray1 |
9+
| RuntimeExecTest.java:25:66:25:71 | script : String | RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String |
10+
| RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String | RuntimeExecTest.java:32:43:32:55 | commandArray2 |
11+
| RuntimeExecTest.java:31:36:31:41 | script : String | RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String |
12+
| RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [<element>] : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String |
13+
| RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) |
14+
| RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [<element>] : String | RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [<element>] : String |
15+
| RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String | RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [<element>] : String |
16+
| RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String | RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String |
17+
| RuntimeExecTest.java:38:52:38:57 | script : String | RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String |
18+
nodes
19+
| RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | semmle.label | getenv(...) : String |
20+
| RuntimeExecTest.java:22:43:22:73 | new String[] | semmle.label | new String[] |
21+
| RuntimeExecTest.java:22:43:22:73 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
22+
| RuntimeExecTest.java:22:67:22:72 | script : String | semmle.label | script : String |
23+
| RuntimeExecTest.java:25:42:25:72 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
24+
| RuntimeExecTest.java:25:66:25:71 | script : String | semmle.label | script : String |
25+
| RuntimeExecTest.java:26:43:26:55 | commandArray1 | semmle.label | commandArray1 |
26+
| RuntimeExecTest.java:31:17:31:29 | commandArray2 [post update] : String[] [[]] : String | semmle.label | commandArray2 [post update] : String[] [[]] : String |
27+
| RuntimeExecTest.java:31:36:31:41 | script : String | semmle.label | script : String |
28+
| RuntimeExecTest.java:32:43:32:55 | commandArray2 | semmle.label | commandArray2 |
29+
| RuntimeExecTest.java:36:21:39:21 | concat(...) : Stream [<element>] : String | semmle.label | concat(...) : Stream [<element>] : String |
30+
| RuntimeExecTest.java:36:21:39:44 | toArray(...) | semmle.label | toArray(...) |
31+
| RuntimeExecTest.java:36:21:39:44 | toArray(...) : String[] [[]] : String | semmle.label | toArray(...) : String[] [[]] : String |
32+
| RuntimeExecTest.java:38:25:38:59 | stream(...) : Stream [<element>] : String | semmle.label | stream(...) : Stream [<element>] : String |
33+
| RuntimeExecTest.java:38:39:38:58 | new String[] : String[] [[]] : String | semmle.label | new String[] : String[] [[]] : String |
34+
| RuntimeExecTest.java:38:39:38:58 | {...} : String[] [[]] : String | semmle.label | {...} : String[] [[]] : String |
35+
| RuntimeExecTest.java:38:52:38:57 | script : String | semmle.label | script : String |
36+
subpaths
37+
#select
38+
| RuntimeExecTest.java:22:43:22:73 | new String[] | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:22:43:22:73 | new String[] | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:22:56:22:64 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String |
39+
| RuntimeExecTest.java:26:43:26:55 | commandArray1 | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:26:43:26:55 | commandArray1 | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:25:55:25:63 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String |
40+
| RuntimeExecTest.java:32:43:32:55 | commandArray2 | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:32:43:32:55 | commandArray2 | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:30:36:30:44 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String |
41+
| RuntimeExecTest.java:36:21:39:44 | toArray(...) | RuntimeExecTest.java:17:25:17:51 | getenv(...) : String | RuntimeExecTest.java:36:21:39:44 | toArray(...) | Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@' | RuntimeExecTest.java:37:52:37:60 | "/bin/sh" | "/bin/sh" | RuntimeExecTest.java:17:25:17:51 | getenv(...) | getenv(...) : String |

‎java/ql/test/experimental/query-tests/security/CWE-078/CommandInjectionRuntimeExecLocal.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-078/CommandInjectionRuntimeExecLocal.ql

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/* Tests for command injection query
2+
*
3+
* This is suitable for testing static analysis tools, as long as they treat local input as an attack surface (which can be prone to false positives)
4+
*
5+
* (C) Copyright GitHub, 2023
6+
*
7+
*/
8+
9+
import java.util.stream.Stream;
10+
import java.io.IOException;
11+
import java.util.Arrays;
12+
13+
public class RuntimeExecTest {
14+
public static void test() {
15+
System.out.println("Command injection test");
16+
17+
String script = System.getenv("SCRIPTNAME");
18+
19+
if (script != null) {
20+
try {
21+
// 1. array literal in the args
22+
Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
23+
24+
// 2. array literal with dataflow
25+
String[] commandArray1 = new String[]{"/bin/sh", script};
26+
Runtime.getRuntime().exec(commandArray1);
27+
28+
// 3. array assignment after it is created
29+
String[] commandArray2 = new String[4];
30+
commandArray2[0] = "/bin/sh";
31+
commandArray2[1] = script;
32+
Runtime.getRuntime().exec(commandArray2);
33+
34+
// 4. Stream concatenation
35+
Runtime.getRuntime().exec(
36+
Stream.concat(
37+
Arrays.stream(new String[]{"/bin/sh"}),
38+
Arrays.stream(new String[]{script})
39+
).toArray(String[]::new)
40+
);
41+
42+
} catch (Exception e) {
43+
System.err.println("ERROR: " + e.getMessage());
44+
}
45+
}
46+
}
47+
}

0 commit comments

Comments
 (0)