Skip to content

Commit 47506a8

Browse files
authored
Merge pull request #4287 from joefarebrother/exectainted-array
Java: Improve the ExecTainted query
2 parents 269b710 + 9baf2b9 commit 47506a8

File tree

11 files changed

+305
-4
lines changed

11 files changed

+305
-4
lines changed

java/ql/src/Security/CWE/CWE-078/ExecCommon.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import semmle.code.java.dataflow.FlowSources
22
import semmle.code.java.security.ExternalProcess
3+
import semmle.code.java.security.CommandArguments
34

45
private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
56
RemoteUserInputToArgumentToExecFlowConfig() {
@@ -11,7 +12,11 @@ private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::C
1112
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
1213

1314
override predicate isSanitizer(DataFlow::Node node) {
14-
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
15+
node.getType() instanceof PrimitiveType
16+
or
17+
node.getType() instanceof BoxedType
18+
or
19+
isSafeCommandArgument(node.asExpr())
1520
}
1621
}
1722

java/ql/src/Security/CWE/CWE-078/ExecTainted.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import semmle.code.java.security.ExternalProcess
1717
import ExecCommon
1818
import DataFlow::PathGraph
1919

20-
from DataFlow::PathNode source, DataFlow::PathNode sink, StringArgumentToExec execArg
20+
from DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg
2121
where execTainted(source, sink, execArg)
2222
select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
2323
"User-provided value"

java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import semmle.code.java.Expr
1515
import semmle.code.java.dataflow.FlowSources
1616
import semmle.code.java.security.ExternalProcess
17+
import semmle.code.java.security.CommandArguments
1718
import DataFlow::PathGraph
1819

1920
class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
@@ -24,12 +25,16 @@ class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configurat
2425
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
2526

2627
override predicate isSanitizer(DataFlow::Node node) {
27-
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
28+
node.getType() instanceof PrimitiveType
29+
or
30+
node.getType() instanceof BoxedType
31+
or
32+
isSafeCommandArgument(node.asExpr())
2833
}
2934
}
3035

3136
from
32-
DataFlow::PathNode source, DataFlow::PathNode sink, StringArgumentToExec execArg,
37+
DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg,
3338
LocalUserInputToArgumentToExecFlowConfig conf
3439
where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = execArg
3540
select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/**
2+
* Definitions for reasoning about lists and arrays that are to be used as arguments to an external process.
3+
*/
4+
5+
import java
6+
import semmle.code.java.dataflow.SSA
7+
import semmle.code.java.Collections
8+
9+
/**
10+
* Holds if `ex` is used safely as an argument to a command;
11+
* i.e. it's not in the first position and it's not a shell command.
12+
*/
13+
predicate isSafeCommandArgument(Expr ex) {
14+
exists(ArrayInit ai, int i |
15+
ex = ai.getInit(i) and
16+
i > 0 and
17+
not isShell(ai.getInit(0))
18+
)
19+
or
20+
exists(CommandArgumentList cal |
21+
not cal.isShell() and
22+
ex = cal.getASubsequentAdd().getArgument(0)
23+
)
24+
or
25+
exists(CommandArgArrayImmutableFirst caa |
26+
not caa.isShell() and
27+
ex = caa.getAWrite(any(int i | i > 0))
28+
)
29+
}
30+
31+
/**
32+
* Holds if the given expression is the name of a shell command such as bash or python
33+
*/
34+
private predicate isShell(Expr ex) {
35+
exists(string cmd | cmd = ex.(StringLiteral).getValue() |
36+
cmd.regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
37+
)
38+
or
39+
exists(SsaVariable ssa |
40+
ex = ssa.getAUse() and
41+
isShell(ssa.getAnUltimateDefinition().(SsaExplicitUpdate).getDefiningExpr())
42+
)
43+
or
44+
isShell(ex.(Assignment).getRhs())
45+
or
46+
isShell(ex.(LocalVariableDeclExpr).getInit())
47+
}
48+
49+
/**
50+
* A type that could be a list of strings. Includes raw `List` types.
51+
*/
52+
private class ListOfStringType extends CollectionType {
53+
ListOfStringType() {
54+
this.getSourceDeclaration().getASourceSupertype*().hasQualifiedName("java.util", "List") and
55+
this.getElementType().getASubtype*() instanceof TypeString
56+
}
57+
}
58+
59+
/**
60+
* A variable that could be used as a list of arguments to a command.
61+
*/
62+
private class CommandArgumentList extends SsaExplicitUpdate {
63+
CommandArgumentList() {
64+
this.getSourceVariable().getType() instanceof ListOfStringType and
65+
forex(CollectionMutation ma | ma.getQualifier() = this.getAUse() |
66+
ma.getMethod().getName().matches("add%")
67+
)
68+
}
69+
70+
/** Gets a use of the variable for which the list could be empty. */
71+
private RValue getAUseBeforeFirstAdd() {
72+
result = getAFirstUse()
73+
or
74+
exists(RValue mid |
75+
mid = getAUseBeforeFirstAdd() and
76+
adjacentUseUse(mid, result) and
77+
not exists(MethodAccess ma |
78+
mid = ma.getQualifier() and
79+
ma.getMethod().hasName("add")
80+
)
81+
)
82+
}
83+
84+
/**
85+
* Gets an addition to this list, i.e. a call to an `add` or `addAll` method.
86+
*/
87+
MethodAccess getAnAdd() {
88+
result.getQualifier() = getAUse() and
89+
result.getMethod().getName().matches("add%")
90+
}
91+
92+
/** Gets an addition to this list which could be its first element. */
93+
MethodAccess getAFirstAdd() {
94+
result = getAnAdd() and
95+
result.getQualifier() = getAUseBeforeFirstAdd()
96+
}
97+
98+
/** Gets an addition to this list which is not the first element. */
99+
MethodAccess getASubsequentAdd() {
100+
result = getAnAdd() and
101+
not result = getAFirstAdd()
102+
}
103+
104+
/** Holds if the first element of this list is a shell command. */
105+
predicate isShell() {
106+
exists(MethodAccess ma | ma = getAFirstAdd() and isShell(ma.getArgument(0)))
107+
}
108+
}
109+
110+
/**
111+
* The type `String[]`.
112+
*/
113+
private class ArrayOfStringType extends Array {
114+
ArrayOfStringType() { this.getElementType() instanceof TypeString }
115+
}
116+
117+
private predicate arrayLValue(ArrayAccess acc) { exists(Assignment a | a.getDest() = acc) }
118+
119+
/**
120+
* A variable that could be an array of arguments to a command.
121+
*/
122+
private class CommandArgumentArray extends SsaExplicitUpdate {
123+
CommandArgumentArray() {
124+
this.getSourceVariable().getType() instanceof ArrayOfStringType and
125+
forall(ArrayAccess a | a.getArray() = getAUse() and arrayLValue(a) |
126+
a.getIndexExpr() instanceof CompileTimeConstantExpr
127+
)
128+
}
129+
130+
/** Gets an expression that is written to the given index of this array at the given use. */
131+
Expr getAWrite(int index, RValue use) {
132+
exists(Assignment a, ArrayAccess acc |
133+
acc.getArray() = use and
134+
use = this.getAUse() and
135+
index = acc.getIndexExpr().(CompileTimeConstantExpr).getIntValue() and
136+
acc = a.getDest() and
137+
result = a.getRhs()
138+
)
139+
}
140+
141+
/** Gets an expression that is written to the given index of this array. */
142+
Expr getAWrite(int index) { result = getAWrite(index, _) }
143+
}
144+
145+
/**
146+
* A `CommandArgArray` whose element at index 0 is never written to, except possibly once to initialise it.
147+
*/
148+
private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
149+
CommandArgArrayImmutableFirst() {
150+
(exists(getAWrite(0)) or exists(firstElementOf(this.getDefiningExpr()))) and
151+
forall(RValue use | exists(this.getAWrite(0, use)) | use = this.getAFirstUse())
152+
}
153+
154+
/** Gets the first element of this array. */
155+
Expr getFirstElement() {
156+
result = getAWrite(0)
157+
or
158+
not exists(getAWrite(0)) and
159+
result = firstElementOf(getDefiningExpr())
160+
}
161+
162+
/** Holds if the first element of this array is a shell command. */
163+
predicate isShell() { isShell(getFirstElement()) }
164+
}
165+
166+
/** Gets the first element of an imutable array of strings */
167+
private Expr firstElementOf(Expr arr) {
168+
arr.getType() instanceof ArrayOfStringType and
169+
(
170+
result = firstElementOf(arr.(Assignment).getRhs())
171+
or
172+
result = firstElementOf(arr.(LocalVariableDeclExpr).getInit())
173+
or
174+
exists(CommandArgArrayImmutableFirst caa | arr = caa.getAUse() | result = caa.getFirstElement())
175+
or
176+
exists(MethodAccess ma, Method m |
177+
arr = ma and
178+
ma.getMethod() = m and
179+
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
180+
m.hasName("copyOf") and
181+
result = firstElementOf(ma.getArgument(0))
182+
)
183+
or
184+
exists(Field f |
185+
f.isStatic() and
186+
arr.(FieldRead).getField() = f and
187+
result = firstElementOf(f.getInitializer())
188+
)
189+
or
190+
result = arr.(ArrayInit).getInit(0)
191+
or
192+
result = arr.(ArrayCreationExpr).getInit().getInit(0)
193+
)
194+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| Test.java:50:46:50:49 | "ls" | Command with a relative path 'ls' is executed. |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-078/ExecRelative.ql
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
edges
2+
| Test.java:6:35:6:44 | arg : String | Test.java:7:44:7:69 | ... + ... |
3+
| Test.java:6:35:6:44 | arg : String | Test.java:10:29:10:74 | new String[] |
4+
| Test.java:6:35:6:44 | arg : String | Test.java:18:29:18:31 | cmd |
5+
| Test.java:6:35:6:44 | arg : String | Test.java:24:29:24:32 | cmd1 |
6+
| Test.java:28:38:28:47 | arg : String | Test.java:29:44:29:64 | ... + ... |
7+
| Test.java:57:27:57:39 | args : String[] | Test.java:60:20:60:22 | arg : String |
8+
| Test.java:57:27:57:39 | args : String[] | Test.java:61:23:61:25 | arg : String |
9+
| Test.java:60:20:60:22 | arg : String | Test.java:6:35:6:44 | arg : String |
10+
| Test.java:61:23:61:25 | arg : String | Test.java:28:38:28:47 | arg : String |
11+
nodes
12+
| Test.java:6:35:6:44 | arg : String | semmle.label | arg : String |
13+
| Test.java:7:44:7:69 | ... + ... | semmle.label | ... + ... |
14+
| Test.java:10:29:10:74 | new String[] | semmle.label | new String[] |
15+
| Test.java:18:29:18:31 | cmd | semmle.label | cmd |
16+
| Test.java:24:29:24:32 | cmd1 | semmle.label | cmd1 |
17+
| Test.java:28:38:28:47 | arg : String | semmle.label | arg : String |
18+
| Test.java:29:44:29:64 | ... + ... | semmle.label | ... + ... |
19+
| Test.java:57:27:57:39 | args : String[] | semmle.label | args : String[] |
20+
| Test.java:60:20:60:22 | arg : String | semmle.label | arg : String |
21+
| Test.java:61:23:61:25 | arg : String | semmle.label | arg : String |
22+
#select
23+
| Test.java:7:44:7:69 | ... + ... | Test.java:57:27:57:39 | args : String[] | Test.java:7:44:7:69 | ... + ... | $@ flows to here and is used in a command. | Test.java:57:27:57:39 | args | User-provided value |
24+
| Test.java:10:29:10:74 | new String[] | Test.java:57:27:57:39 | args : String[] | Test.java:10:29:10:74 | new String[] | $@ flows to here and is used in a command. | Test.java:57:27:57:39 | args | User-provided value |
25+
| Test.java:18:29:18:31 | cmd | Test.java:57:27:57:39 | args : String[] | Test.java:18:29:18:31 | cmd | $@ flows to here and is used in a command. | Test.java:57:27:57:39 | args | User-provided value |
26+
| Test.java:24:29:24:32 | cmd1 | Test.java:57:27:57:39 | args : String[] | Test.java:24:29:24:32 | cmd1 | $@ flows to here and is used in a command. | Test.java:57:27:57:39 | args | User-provided value |
27+
| Test.java:29:44:29:64 | ... + ... | Test.java:57:27:57:39 | args : String[] | Test.java:29:44:29:64 | ... + ... | $@ flows to here and is used in a command. | Test.java:57:27:57:39 | args | User-provided value |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-078/ExecTaintedLocal.ql
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| Test.java:7:44:7:69 | ... + ... | Command line is built with string concatenation. |
2+
| Test.java:29:44:29:64 | ... + ... | Command line is built with string concatenation. |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-078/ExecUnescaped.ql

0 commit comments

Comments
 (0)