Skip to content

Commit 12e0234

Browse files
artem-smotrakovartem-smotrakov
committed
Java: Added CompiledAccExpression sink for MVEL injections
1 parent 32ff5ad commit 12e0234

5 files changed

Lines changed: 83 additions & 24 deletions

File tree

‎java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
1717
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
1818
expressionCompilationStep(node1, node2) or
1919
createExpressionCompilerStep(node1, node2) or
20-
expressionCompilerCompileStep(node1, node2)
20+
expressionCompilerCompileStep(node1, node2) or
21+
createCompiledAccExpressionStep(node1, node2)
2122
}
2223
}
2324

@@ -33,12 +34,11 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
3334
)
3435
or
3536
exists(MethodAccess ma, Method m | m = ma.getMethod() |
36-
m instanceof ExecutableStatementEvaluationMethod and
37-
(ma = asExpr() or ma.getQualifier() = asExpr())
38-
)
39-
or
40-
exists(MethodAccess ma, Method m | m = ma.getMethod() |
41-
m instanceof CompiledExpressionEvaluationMethod and
37+
(
38+
m instanceof ExecutableStatementEvaluationMethod or
39+
m instanceof CompiledExpressionEvaluationMethod or
40+
m instanceof CompiledAccExpressionEvaluationMethod
41+
) and
4242
(ma = asExpr() or ma.getQualifier() = asExpr())
4343
)
4444
}
@@ -69,6 +69,22 @@ predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node
6969
)
7070
}
7171

72+
/**
73+
* Holds if `node1` to `node2` is a dataflow step creates `CompiledAccExpression`,
74+
* i.e. `new CompiledAccExpression(tainted, ...)`.
75+
*/
76+
predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) {
77+
exists(ConstructorCall cc |
78+
cc.getConstructedType() instanceof CompiledAccExpression and
79+
(cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
80+
cc.getArgument(0) = node1.asExpr()
81+
)
82+
}
83+
84+
predicate test() {
85+
exists(ConstructorCall cc | cc.getConstructedType() instanceof CompiledAccExpression)
86+
}
87+
7288
/**
7389
* Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
7490
* by calling `ExpressionCompiler.compile()`.
@@ -133,6 +149,16 @@ class CompiledExpressionEvaluationMethod extends Method {
133149
}
134150
}
135151

152+
/**
153+
* Methods in `CompiledAccExpression` that trigger evaluating a MVEL expression.
154+
*/
155+
class CompiledAccExpressionEvaluationMethod extends Method {
156+
CompiledAccExpressionEvaluationMethod() {
157+
getDeclaringType() instanceof CompiledAccExpression and
158+
hasName("getValue")
159+
}
160+
}
161+
136162
class MVEL extends RefType {
137163
MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
138164
}
@@ -146,5 +172,9 @@ class ExecutableStatement extends RefType {
146172
}
147173

148174
class CompiledExpression extends RefType {
149-
CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
175+
CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
176+
}
177+
178+
class CompiledAccExpression extends RefType {
179+
CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") }
150180
}

‎java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
edges
2-
| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input |
3-
| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression |
4-
| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement |
5-
| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression |
2+
| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input |
3+
| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression |
4+
| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement |
5+
| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression |
6+
| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression |
67
nodes
7-
| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
8-
| MvelInjection.java:18:17:18:21 | input | semmle.label | input |
9-
| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
10-
| MvelInjection.java:28:30:28:39 | expression | semmle.label | expression |
11-
| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
12-
| MvelInjection.java:39:7:39:15 | statement | semmle.label | statement |
13-
| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
14-
| MvelInjection.java:50:7:50:16 | expression | semmle.label | expression |
8+
| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
9+
| MvelInjection.java:20:17:20:21 | input | semmle.label | input |
10+
| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
11+
| MvelInjection.java:30:30:30:39 | expression | semmle.label | expression |
12+
| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
13+
| MvelInjection.java:41:7:41:15 | statement | semmle.label | statement |
14+
| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
15+
| MvelInjection.java:52:7:52:16 | expression | semmle.label | expression |
16+
| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
17+
| MvelInjection.java:62:7:62:16 | expression | semmle.label | expression |
1518
#select
16-
| MvelInjection.java:18:17:18:21 | input | MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input | MVEL injection from $@. | MvelInjection.java:14:27:14:49 | getInputStream(...) | this user input |
17-
| MvelInjection.java:28:30:28:39 | expression | MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression | MVEL injection from $@. | MvelInjection.java:23:27:23:49 | getInputStream(...) | this user input |
18-
| MvelInjection.java:39:7:39:15 | statement | MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement | MVEL injection from $@. | MvelInjection.java:33:27:33:49 | getInputStream(...) | this user input |
19-
| MvelInjection.java:50:7:50:16 | expression | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
19+
| MvelInjection.java:20:17:20:21 | input | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input | MVEL injection from $@. | MvelInjection.java:16:27:16:49 | getInputStream(...) | this user input |
20+
| MvelInjection.java:30:30:30:39 | expression | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
21+
| MvelInjection.java:41:7:41:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
22+
| MvelInjection.java:52:7:52:16 | expression | MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression | MVEL injection from $@. | MvelInjection.java:46:27:46:49 | getInputStream(...) | this user input |
23+
| MvelInjection.java:62:7:62:16 | expression | MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression | MVEL injection from $@. | MvelInjection.java:57:27:57:49 | getInputStream(...) | this user input |

‎java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import java.io.Serializable;
44
import java.net.Socket;
55
import org.mvel2.MVEL;
6+
import org.mvel2.ParserContext;
7+
import org.mvel2.compiler.CompiledAccExpression;
68
import org.mvel2.compiler.CompiledExpression;
79
import org.mvel2.compiler.ExecutableStatement;
810
import org.mvel2.compiler.ExpressionCompiler;
@@ -50,4 +52,14 @@ public static void testWithCompiledExpressionGetDirectValue(Socket socket) throw
5052
expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
5153
}
5254
}
55+
56+
public static void testCompiledAccExpressionGetValue(Socket socket) throws IOException {
57+
try (InputStream in = socket.getInputStream()) {
58+
byte[] bytes = new byte[1024];
59+
int n = in.read(bytes);
60+
String input = new String(bytes, 0, n);
61+
CompiledAccExpression expression = new CompiledAccExpression(input.toCharArray(), Object.class, new ParserContext());
62+
expression.getValue(new Object(), new ImmutableDefaultFactory());
63+
}
64+
}
5365
}

‎java/ql/test/stubs/mvel2-2.4.7/org/mvel2/ParserContext.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package org.mvel2;
2+
3+
public class ParserContext {}

‎java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledAccExpression.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package org.mvel2.compiler;
2+
3+
import org.mvel2.ParserContext;
4+
import org.mvel2.integration.VariableResolverFactory;
5+
6+
public class CompiledAccExpression implements ExecutableStatement {
7+
public CompiledAccExpression(char[] expression, Class ingressType, ParserContext context) {}
8+
public Object getValue(Object staticContext, VariableResolverFactory factory) { return null; }
9+
public Object getValue(Object ctx, Object elCtx, VariableResolverFactory variableFactory) { return null; }
10+
}

0 commit comments

Comments
 (0)