Skip to content

Commit 01798f6

Browse files
aegilopsaegilops
committed
Switched to new dataflow and added a test (but it doesn't produce results yet)
1 parent 23bf847 commit 01798f6

7 files changed

Lines changed: 123 additions & 115 deletions

File tree

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,16 @@
1111
* external/cwe/cwe-078
1212
*/
1313

14-
import DataFlow::PathGraph
1514
import CommandInjectionRuntimeExec
15+
import ExecUserFlow::PathGraph
1616

17-
class RemoteSource extends Source instanceof RemoteFlowSource {}
17+
class RemoteSource extends Source instanceof RemoteFlowSource { }
1818

1919
from
20-
DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf,
21-
MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd,
22-
ExecTaintConfiguration confCmd
20+
ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink,
21+
MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd
2322
where
24-
call.getMethod() instanceof RuntimeExecMethod and
25-
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
26-
(
27-
confCmd.hasFlow(sourceCmd, sinkCmd) and
28-
sinkCmd.asExpr() = call.getArgument(0)
29-
) and
30-
// it is tainted by untrusted user input
31-
(
32-
conf.hasFlow(source.getNode(), sink.getNode()) and
33-
sink.getNode().asExpr() = call.getArgument(0)
34-
)
23+
callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd)
3524
select sink, source, sink,
3625
"Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
3726
sourceCmd, sourceCmd.toString(), source.getNode(), source.toString()

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

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import java
22
import semmle.code.java.frameworks.javaee.ejb.EJBRestrictions
33
import semmle.code.java.dataflow.DataFlow
4-
private import semmle.code.java.dataflow.TaintTracking
54
import semmle.code.java.dataflow.FlowSources
65

76
// a static string of an unsafe executable tainting arg 0 of Runtime.exec()
8-
class ExecTaintConfiguration extends TaintTracking::Configuration {
7+
deprecated class ExecTaintConfiguration extends TaintTracking::Configuration {
98
ExecTaintConfiguration() { this = "ExecTaintConfiguration" }
109

1110
override predicate isSource(DataFlow::Node source) {
@@ -33,12 +32,41 @@ class ExecTaintConfiguration extends TaintTracking::Configuration {
3332
}
3433
}
3534

35+
module ExecCmdFlowConfig implements DataFlow::ConfigSig {
36+
predicate isSource(DataFlow::Node source) {
37+
source.asExpr() instanceof StringLiteral and
38+
source.asExpr().(StringLiteral).getValue() instanceof UnSafeExecutable
39+
}
40+
41+
predicate isSink(DataFlow::Node sink) {
42+
exists(RuntimeExecMethod method, MethodAccess call |
43+
call.getMethod() = method and
44+
sink.asExpr() = call.getArgument(0) and
45+
sink.asExpr().getType() instanceof Array
46+
)
47+
}
48+
49+
predicate isBarrier(DataFlow::Node node) {
50+
node.asExpr().getFile().isSourceFile() and
51+
(
52+
node instanceof AssignToNonZeroIndex or
53+
node instanceof ArrayInitAtNonZeroIndex or
54+
node instanceof StreamConcatAtNonZeroIndex or
55+
node.getType() instanceof PrimitiveType or
56+
node.getType() instanceof BoxedType
57+
)
58+
}
59+
}
60+
61+
/** Tracks flow of unvalidated user input that is used in Runtime.Exec */
62+
module ExecCmdFlow = TaintTracking::Global<ExecCmdFlowConfig>;
63+
3664
abstract class Source extends DataFlow::Node {
3765
Source() { this = this }
3866
}
3967

4068
// taint flow from user data to args of the command
41-
class ExecTaintConfiguration2 extends TaintTracking::Configuration {
69+
deprecated class ExecTaintConfiguration2 extends TaintTracking::Configuration {
4270
ExecTaintConfiguration2() { this = "ExecTaintConfiguration2" }
4371

4472
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -60,6 +88,31 @@ class ExecTaintConfiguration2 extends TaintTracking::Configuration {
6088
}
6189
}
6290

91+
module ExecUserFlowConfig implements DataFlow::ConfigSig {
92+
predicate isSource(DataFlow::Node source) {
93+
source instanceof Source
94+
}
95+
96+
predicate isSink(DataFlow::Node sink) {
97+
exists(RuntimeExecMethod method, MethodAccess call |
98+
call.getMethod() = method and
99+
sink.asExpr() = call.getArgument(_) and
100+
sink.asExpr().getType() instanceof Array
101+
)
102+
}
103+
104+
predicate isBarrier(DataFlow::Node node) {
105+
node.asExpr().getFile().isSourceFile() and
106+
(
107+
node.getType() instanceof PrimitiveType or
108+
node.getType() instanceof BoxedType
109+
)
110+
}
111+
}
112+
113+
/** Tracks flow of unvalidated user input that is used in Runtime.Exec */
114+
module ExecUserFlow = TaintTracking::Global<ExecUserFlowConfig>;
115+
63116
// array[3] = node
64117
class AssignToNonZeroIndex extends DataFlow::Node {
65118
AssignToNonZeroIndex() {
@@ -101,3 +154,17 @@ class UnSafeExecutable extends string {
101154
not this = "netsh.exe"
102155
}
103156
}
157+
158+
predicate callIsTaintedByUserInputAndDangerousCommand(MethodAccess call, ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd) {
159+
call.getMethod() instanceof RuntimeExecMethod and
160+
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
161+
(
162+
ExecCmdFlow::flow(sourceCmd, sinkCmd) and
163+
sinkCmd.asExpr() = call.getArgument(0)
164+
) and
165+
// it is tainted by untrusted user input
166+
(
167+
ExecUserFlow::flowPath(source, sink) and
168+
sink.getNode().asExpr() = call.getArgument(0)
169+
)
170+
}

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,16 @@
1212
* external/cwe/cwe-078
1313
*/
1414

15-
import DataFlow::PathGraph
1615
import CommandInjectionRuntimeExec
16+
import ExecUserFlow::PathGraph
1717

18-
class LocalSource extends Source instanceof LocalUserInput {}
18+
class LocalSource extends Source instanceof LocalUserInput { }
1919

2020
from
21-
DataFlow::PathNode source, DataFlow::PathNode sink, ExecTaintConfiguration2 conf,
22-
MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd,
23-
ExecTaintConfiguration confCmd
21+
ExecUserFlow::PathNode source, ExecUserFlow::PathNode sink,
22+
MethodAccess call, DataFlow::Node sourceCmd, DataFlow::Node sinkCmd
2423
where
25-
call.getMethod() instanceof RuntimeExecMethod and
26-
// this is a command-accepting call to exec, e.g. rt.exec(new String[]{"/bin/sh", ...})
27-
(
28-
confCmd.hasFlow(sourceCmd, sinkCmd) and
29-
sinkCmd.asExpr() = call.getArgument(0)
30-
) and
31-
// it is tainted by untrusted user input
32-
(
33-
conf.hasFlow(source.getNode(), sink.getNode()) and
34-
sink.getNode().asExpr() = call.getArgument(0)
35-
)
24+
callIsTaintedByUserInputAndDangerousCommand(call, source, sink, sourceCmd, sinkCmd)
3625
select sink, source, sink,
3726
"Call to dangerous java.lang.Runtime.exec() with command '$@' with arg from untrusted input '$@'",
3827
sourceCmd, sourceCmd.toString(), source.getNode(), source.toString()

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

Lines changed: 0 additions & 39 deletions
This file was deleted.

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

Lines changed: 0 additions & 41 deletions
This file was deleted.

‎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: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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(String[] args) {
15+
System.out.println("Command injection test");
16+
17+
try {
18+
// 1. array literal
19+
String[] commandArray1 = new String[]{"/bin/sh", args[2], args[3], args[4]};
20+
Runtime.getRuntime().exec(commandArray1);
21+
22+
// 2. array assignment after it is created
23+
String[] commandArray2 = new String[4];
24+
commandArray2[0] = "/bin/sh";
25+
commandArray2[1] = args[2];
26+
commandArray2[2] = args[3];
27+
commandArray2[3] = args[4];
28+
Runtime.getRuntime().exec(commandArray2);
29+
30+
// 3. Stream concatenation
31+
Runtime.getRuntime().exec(
32+
Stream.concat(
33+
Arrays.stream(new String[]{"/bin/sh"}),
34+
Arrays.stream(new String[]{args[2], args[3], args[4]})
35+
).toArray(String[]::new)
36+
);
37+
38+
} catch (Exception e) {
39+
System.err.println("ERROR: " + e.getMessage());
40+
}
41+
}
42+
}

0 commit comments

Comments
 (0)