Skip to content

Commit b6c35dd

Browse files
aegilopsaegilops
committed
Added experimental version of Java Command Injection query, to be more sensitive to unusual code constructs
1 parent b572974 commit b6c35dd

10 files changed

+423
-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.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Command Injection into Runtime.exec() with dangerous command
2+
Code that passes remote user input to an arugment of a call of `Runtime.exec` that executes a scripting executable will allow the user to execute malicious code.
3+
4+
5+
## Recommendation
6+
If possible, use hard-coded string literals to specify the command or script to run, or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
7+
8+
If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
9+
10+
11+
## Example
12+
The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to the array going into `Runtime.exec` without examining it first.
13+
14+
15+
```java
16+
class Test {
17+
public static void main(String[] args) {
18+
String script = System.getenv("SCRIPTNAME");
19+
if (script != null) {
20+
// BAD: The script to be executed by /bin/sh is controlled by the user.
21+
Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
22+
}
23+
}
24+
}
25+
```
26+
27+
## References
28+
* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
29+
* SEI CERT Oracle Coding Standard for Java: [IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method](https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method).
30+
* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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+
41+
42+
<!-- LocalWords: CWE untrusted unsanitized Runtime
43+
-->
44+
45+
</references>
46+
</qhelp>

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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+
* external/cwe/cwe-078
11+
*/
12+
13+
14+
import DataFlow::PathGraph
15+
import CommandInjectionRuntimeExec
16+
17+
class RemoteSource extends Source { RemoteSource() { this instanceof RemoteFlowSource } }
18+
19+
from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
20+
where call.getMethod() instanceof RuntimeExecMethod
21+
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
22+
and (
23+
confCmd.hasFlow(sourceCmd, sinkCmd)
24+
and sinkCmd.asExpr() = call.getArgument(0)
25+
)
26+
// it is tainted by untrusted user input
27+
and (
28+
conf.hasFlow(source.getNode(), sink.getNode())
29+
and sink.getNode().asExpr() = call.getArgument(0)
30+
)
31+
select sink, source, sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
32+
sourceCmd, sourceCmd.toString(),
33+
source.getNode(), source.toString()

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import java
2+
import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
3+
import semmle.code.java.dataflow.DataFlow
4+
private import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.dataflow.FlowSources
6+
7+
8+
// a static string of an unsafe executable tainting arg 0 of Runtime.exec()
9+
class ExecTaintConfiguration extends TaintTracking::Configuration {
10+
ExecTaintConfiguration() { this = "ExecTaintConfiguration" }
11+
12+
override
13+
predicate
14+
isSource(DataFlow::Node source) {
15+
source.asExpr() instanceof StringLiteral
16+
and source.asExpr().(StringLiteral).getValue() instanceof UnSafeExecutable
17+
}
18+
19+
override
20+
predicate
21+
isSink(DataFlow::Node sink) {
22+
exists(RuntimeExecMethod method, MethodAccess call |
23+
call.getMethod() = method
24+
and sink.asExpr() = call.getArgument(0)
25+
and sink.asExpr().getType() instanceof Array
26+
)
27+
}
28+
29+
override
30+
predicate
31+
isSanitizer(DataFlow::Node node) {
32+
node.asExpr().getFile().isSourceFile() and
33+
(
34+
node instanceof AssignToNonZeroIndex
35+
or node instanceof ArrayInitAtNonZeroIndex
36+
or node instanceof StreamConcatAtNonZeroIndex
37+
or node.getType() instanceof PrimitiveType
38+
or node.getType() instanceof BoxedType
39+
)
40+
}
41+
}
42+
43+
abstract class Source extends DataFlow::Node {
44+
Source() {
45+
this = this
46+
}
47+
}
48+
49+
50+
// taint flow from user data to args of the command
51+
class ExecTaintConfiguration2 extends TaintTracking::Configuration {
52+
ExecTaintConfiguration2() { this = "ExecTaintConfiguration2" }
53+
54+
override
55+
predicate
56+
isSource(DataFlow::Node source) {
57+
source instanceof Source
58+
}
59+
60+
override
61+
predicate
62+
isSink(DataFlow::Node sink) {
63+
exists(RuntimeExecMethod method, MethodAccess call, int index |
64+
call.getMethod() = method
65+
and sink.asExpr() = call.getArgument(index)
66+
and sink.asExpr().getType() instanceof Array
67+
)
68+
}
69+
70+
override
71+
predicate
72+
isSanitizer(DataFlow::Node node) {
73+
node.asExpr().getFile().isSourceFile() and
74+
(
75+
node.getType() instanceof PrimitiveType
76+
or node.getType() instanceof BoxedType
77+
)
78+
}
79+
}
80+
81+
82+
// array[3] = node
83+
class AssignToNonZeroIndex extends DataFlow::Node {
84+
AssignExpr assign;
85+
ArrayAccess access;
86+
87+
AssignToNonZeroIndex() {
88+
assign.getDest() = access
89+
and access.getIndexExpr().(IntegerLiteral).getValue() != "0"
90+
and assign.getSource() = this.asExpr()
91+
}
92+
}
93+
94+
95+
// String[] array = {"a", "b, "c"};
96+
class ArrayInitAtNonZeroIndex extends DataFlow::Node {
97+
ArrayInit init;
98+
int index;
99+
100+
ArrayInitAtNonZeroIndex() {
101+
init.getInit(index) = this.asExpr()
102+
and index != 0
103+
}
104+
}
105+
106+
// Stream.concat(Arrays.stream(array_1), Arrays.stream(array_2))
107+
class StreamConcatAtNonZeroIndex extends DataFlow::Node {
108+
MethodAccess call;
109+
int index;
110+
111+
StreamConcatAtNonZeroIndex() {
112+
call.getMethod().getQualifiedName() = "java.util.stream.Stream.concat"
113+
and call.getArgument(index) = this.asExpr()
114+
and index != 0
115+
}
116+
}
117+
118+
119+
// allow list of executables that execute their arguments
120+
// TODO: extend with data extensions
121+
class UnSafeExecutable extends string {
122+
bindingset[this]
123+
UnSafeExecutable() {
124+
this.regexpMatch("^(|.*/)([a-z]*sh|javac?|python.*|perl|[Pp]ower[Ss]hell|php|node|deno|bun|ruby|osascript|cmd|Rscript|groovy)(\\.exe)?$")
125+
and not this.matches("netsh.exe")
126+
}
127+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Command Injection into Runtime.exec() with dangerous command
2+
Code that passes local user input to an arugment of a call of `Runtime.exec` that executes a scripting executable will allow the user to execute malicious code.
3+
4+
5+
## Recommendation
6+
If possible, use hard-coded string literals to specify the command or script to run, or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
7+
8+
If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
9+
10+
11+
## Example
12+
The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to the array going into `Runtime.exec` without examining it first.
13+
14+
15+
```java
16+
class Test {
17+
public static void main(String[] args) {
18+
String script = System.getenv("SCRIPTNAME");
19+
if (script != null) {
20+
// BAD: The script to be executed by /bin/sh is controlled by the user.
21+
Runtime.getRuntime().exec(new String[]{"/bin/sh", script});
22+
}
23+
}
24+
}
25+
```
26+
27+
## References
28+
* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
29+
* SEI CERT Oracle Coding Standard for Java: [IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method](https://wiki.sei.cmu.edu/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec()+method).
30+
* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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+
41+
42+
<!-- LocalWords: CWE untrusted unsanitized local Runtime
43+
-->
44+
45+
</references>
46+
</qhelp>

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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+
* local
11+
* external/cwe/cwe-078
12+
*/
13+
14+
15+
import DataFlow::PathGraph
16+
import CommandInjectionRuntimeExec
17+
18+
class LocalSource extends Source { LocalSource() { this instanceof LocalUserInput } }
19+
20+
from DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf, MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
21+
where call.getMethod() instanceof RuntimeExecMethod
22+
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
23+
and (
24+
confCmd.hasFlow(sourceCmd, sinkCmd)
25+
and sinkCmd.asExpr() = call.getArgument(0)
26+
)
27+
// it is tainted by untrusted user input
28+
and (
29+
conf.hasFlow(source.getNode(), sink.getNode())
30+
and sink.getNode().asExpr() = call.getArgument(0)
31+
)
32+
select sink, source, sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
33+
sourceCmd, sourceCmd.toString(),
34+
source.getNode(), source.toString()

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* @name Command Injection into Runtime.exec() with dangerous command
3+
* @description Testing query. 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 problem
5+
* @problem.severity error
6+
* @security-severity 6.1
7+
* @precision high
8+
* @id java/command-line-injection-extra-test
9+
* @tags testing
10+
* test
11+
* security
12+
* external/cwe/cwe-078
13+
*/
14+
15+
16+
import CommandInjectionRuntimeExec
17+
18+
class DataSource extends Source { DataSource() { this instanceof RemoteFlowSource or this instanceof LocalUserInput } }
19+
20+
from DataFlow::Node source, DataFlow::Node sink, ExecTaintConfiguration2 conf, MethodAccess call, int index, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd, ExecTaintConfiguration confCmd
21+
where call.getMethod() instanceof RuntimeExecMethod
22+
// this is a command-accepting call to exec, e.g. exec("/bin/sh", ...)
23+
and (
24+
confCmd.hasFlow(sourceCmd, sinkCmd)
25+
and sinkCmd.asExpr() = call.getArgument(0)
26+
)
27+
// it is tainted by untrusted user input
28+
and (
29+
conf.hasFlow(source, sink)
30+
and sink.asExpr() = call.getArgument(index)
31+
)
32+
select sink, "Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
33+
sourceCmd, sourceCmd.toString(),
34+
source, source.toString()

0 commit comments

Comments
 (0)