Skip to content

Commit c6c4c2c

Browse files
artem-smotrakovartem-smotrakov
committed
Java: Add a query for MVEL injections
- Added experimental/Security/CWE/CWE-094/MvelInjection.ql - Added experimental/Security/CWE/CWE-094/MvelInjectionLib.qll - Added a qhelp file with an example of vulnerable code - Added tests and stubs for mvel2-2.4.7
1 parent e5480e4 commit c6c4c2c

File tree

13 files changed

+285
-1
lines changed

13 files changed

+285
-1
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
MVEL is an expression language based on Java-syntax.
7+
The language offers many features
8+
including invocation of methods available in the JVM.
9+
If a MVEL expression is built using attacker-controlled data,
10+
and then evaluated, then it may allow the attacker to run arbitrary code.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Including user input in a MVEL expression should be avoided.
17+
</p>
18+
</recommendation>
19+
20+
<example>
21+
<p>
22+
The following example uses untrusted data to build a MVEL expression
23+
and then runs it in the default powerfull context.
24+
</p>
25+
<sample src="UnsafeMvelExpressionEvaluation.java" />
26+
27+
<references>
28+
<li>
29+
MVEL Documentation:
30+
<a href="http://mvel.documentnode.com/">Language Guide for 2.0</a>.
31+
</li>
32+
<li>
33+
OWASP:
34+
<a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
35+
</li>
36+
</references>
37+
</qhelp>

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Expression language injection (MVEL)
3+
* @description Evaluation of a user-controlled MVEL expression
4+
* may lead to remote code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/mvel-expression-injection
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import MvelInjectionLib
15+
import DataFlow::PathGraph
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionConfig conf
18+
where conf.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input"

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.dataflow.TaintTracking
4+
import DataFlow::PathGraph
5+
6+
/**
7+
* A taint-tracking configuration for unsafe user input
8+
* that is used to construct and evaluate a MVEL expression.
9+
*/
10+
class MvelInjectionConfig extends TaintTracking::Configuration {
11+
MvelInjectionConfig() { this = "MvelInjectionConfig" }
12+
13+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
14+
15+
override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
16+
17+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
18+
expressionCompilationStep(node1, node2) or
19+
createExpressionCompilerStep(node1, node2) or
20+
expressionCompilerCompileStep(node1, node2)
21+
}
22+
}
23+
24+
/**
25+
* A sink for EL injection vulnerabilities via MVEL,
26+
* i.e. methods that run evaluation of a MVEL expression.
27+
*/
28+
class MvelEvaluationSink extends DataFlow::ExprNode {
29+
MvelEvaluationSink() {
30+
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
31+
m instanceof MvelEvalMethod and
32+
ma.getAnArgument() = asExpr()
33+
)
34+
or
35+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
36+
m instanceof ExecutableStatementEvaluationMethod and
37+
(ma = asExpr() or ma.getQualifier() = asExpr())
38+
)
39+
}
40+
}
41+
42+
/**
43+
* Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
44+
* by callilng `MVEL.compileExpression(tainted)`.
45+
*/
46+
predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2) {
47+
exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
48+
m.getDeclaringType() instanceof MVEL and
49+
m.hasName("compileExpression") and
50+
ma.getAnArgument() = node1.asExpr() and
51+
(node2.asExpr() = ma.getQualifier() or node2.asExpr() = ma)
52+
)
53+
}
54+
55+
/**
56+
* Holds if `node1` to `node2` is a dataflow step creates `ExpressionCompiler`,
57+
* i.e. `new ExpressionCompiler(tainted)`.
58+
*/
59+
predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
60+
exists(ConstructorCall cc |
61+
cc.getConstructedType() instanceof ExpressionCompiler and
62+
(cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
63+
cc.getArgument(0) = node1.asExpr()
64+
)
65+
}
66+
67+
/**
68+
* Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
69+
* by calling `ExpressionCompiler.compile()`.
70+
*/
71+
predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
72+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
73+
m.getDeclaringType() instanceof ExpressionCompiler and
74+
m.hasName("compile") and
75+
ma = node2.asExpr() and
76+
ma.getQualifier() = node1.asExpr()
77+
)
78+
}
79+
80+
/**
81+
* Methods in the MVEL class that evaluate a MVEL expression.
82+
*/
83+
class MvelEvalMethod extends Method {
84+
MvelEvalMethod() {
85+
getDeclaringType() instanceof MVEL and
86+
(
87+
hasName("eval") or
88+
hasName("executeExpression") or
89+
hasName("evalToBoolean") or
90+
hasName("evalToString") or
91+
hasName("executeAllExpression") or
92+
hasName("executeSetExpression")
93+
)
94+
}
95+
}
96+
97+
/**
98+
* Methods in `MVEL` class that compile a MVEL expression.
99+
*/
100+
class MvelCompileExpressionMethod extends Method {
101+
MvelCompileExpressionMethod() {
102+
getDeclaringType() instanceof MVEL and
103+
(
104+
hasName("compileExpression") or
105+
hasName("compileGetExpression") or
106+
hasName("compileSetExpression")
107+
)
108+
}
109+
}
110+
111+
/**
112+
* Methods in `ExecutableStatement` that trigger evaluating a MVEL expression.
113+
*/
114+
class ExecutableStatementEvaluationMethod extends Method {
115+
ExecutableStatementEvaluationMethod() {
116+
getDeclaringType() instanceof ExecutableStatement and
117+
hasName("getValue")
118+
}
119+
}
120+
121+
class MVEL extends RefType {
122+
MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
123+
}
124+
125+
class ExpressionCompiler extends RefType {
126+
ExpressionCompiler() { hasQualifiedName("org.mvel2.compiler", "ExpressionCompiler") }
127+
}
128+
129+
class ExecutableStatement extends RefType {
130+
ExecutableStatement() { hasQualifiedName("org.mvel2.compiler", "ExecutableStatement") }
131+
}

‎java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
public void evaluate(Socket socket) throws IOException {
2+
try (BufferedReader reader = new BufferedReader(
3+
new InputStreamReader(socket.getInputStream()))) {
4+
5+
String expression = reader.readLine();
6+
MVEL.eval(expression);
7+
}
8+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
edges
2+
| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input |
3+
| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression |
4+
| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement |
5+
nodes
6+
| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
7+
| MvelInjection.java:17:17:17:21 | input | semmle.label | input |
8+
| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
9+
| MvelInjection.java:27:30:27:39 | expression | semmle.label | expression |
10+
| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
11+
| MvelInjection.java:38:7:38:15 | statement | semmle.label | statement |
12+
#select
13+
| MvelInjection.java:17:17:17:21 | input | MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input | MVEL injection from $@. | MvelInjection.java:13:27:13:49 | getInputStream(...) | this user input |
14+
| MvelInjection.java:27:30:27:39 | expression | MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression | MVEL injection from $@. | MvelInjection.java:22:27:22:49 | getInputStream(...) | this user input |
15+
| MvelInjection.java:38:7:38:15 | statement | MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement | MVEL injection from $@. | MvelInjection.java:32:27:32:49 | getInputStream(...) | this user input |

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import java.io.IOException;
2+
import java.io.InputStream;
3+
import java.io.Serializable;
4+
import java.net.Socket;
5+
import org.mvel2.MVEL;
6+
import org.mvel2.compiler.ExecutableStatement;
7+
import org.mvel2.compiler.ExpressionCompiler;
8+
import org.mvel2.integration.impl.ImmutableDefaultFactory;
9+
10+
public class MvelInjection {
11+
12+
public static void testWithMvelEval(Socket socket) throws IOException {
13+
try (InputStream in = socket.getInputStream()) {
14+
byte[] bytes = new byte[1024];
15+
int n = in.read(bytes);
16+
String input = new String(bytes, 0, n);
17+
MVEL.eval(input);
18+
}
19+
}
20+
21+
public static void testWithMvelCompileAndExecute(Socket socket) throws IOException {
22+
try (InputStream in = socket.getInputStream()) {
23+
byte[] bytes = new byte[1024];
24+
int n = in.read(bytes);
25+
String input = new String(bytes, 0, n);
26+
Serializable expression = MVEL.compileExpression(input);
27+
MVEL.executeExpression(expression);
28+
}
29+
}
30+
31+
public static void testWithExpressionCompiler(Socket socket) throws IOException {
32+
try (InputStream in = socket.getInputStream()) {
33+
byte[] bytes = new byte[1024];
34+
int n = in.read(bytes);
35+
String input = new String(bytes, 0, n);
36+
ExpressionCompiler compiler = new ExpressionCompiler(input);
37+
ExecutableStatement statement = compiler.compile();
38+
statement.getValue(new Object(), new ImmutableDefaultFactory());
39+
}
40+
}
41+
}

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

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

‎java/ql/test/experimental/Security/CWE/CWE-094/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package org.mvel2;
2+
3+
import java.io.Serializable;
4+
5+
public class MVEL {
6+
public static Object eval(String expression) { return null; }
7+
public static Serializable compileExpression(String expression) { return null; }
8+
public static Object executeExpression(Object compiledExpression) { return null; }
9+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package org.mvel2.compiler;
2+
3+
import org.mvel2.integration.VariableResolverFactory;
4+
5+
public interface ExecutableStatement {
6+
public Object getValue(Object staticContext, VariableResolverFactory factory);
7+
}

0 commit comments

Comments
 (0)