Skip to content

Commit 8fd7265

Browse files
artem-smotrakovartem-smotrakov
committed
Java: Added JSR 223 sinks for MVEL injections
- Updated MvelInjectionLib.qll - Added tests and stubs for JSR 223 API
1 parent 6a6c805 commit 8fd7265

File tree

11 files changed

+199
-32
lines changed

11 files changed

+199
-32
lines changed

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

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
1818
expressionCompilationStep(node1, node2) or
1919
createExpressionCompilerStep(node1, node2) or
2020
expressionCompilerCompileStep(node1, node2) or
21-
createCompiledAccExpressionStep(node1, node2)
21+
createCompiledAccExpressionStep(node1, node2) or
22+
scriptCompileStep(node1, node2) or
23+
createMvelCompiledScriptStep(node1, node2)
2224
}
2325
}
2426

@@ -30,15 +32,22 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
3032
MvelEvaluationSink() {
3133
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
3234
m instanceof MvelEvalMethod and
33-
ma.getAnArgument() = asExpr()
35+
ma.getArgument(0) = asExpr()
36+
)
37+
or
38+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
39+
m instanceof MvelScriptEngineEvaluationMethod and
40+
ma.getArgument(0) = asExpr()
3441
)
3542
or
3643
exists(MethodAccess ma, Method m | m = ma.getMethod() |
3744
(
3845
m instanceof ExecutableStatementEvaluationMethod or
3946
m instanceof CompiledExpressionEvaluationMethod or
4047
m instanceof CompiledAccExpressionEvaluationMethod or
41-
m instanceof AccessorEvaluationMethod
48+
m instanceof AccessorEvaluationMethod or
49+
m instanceof CompiledScriptEvaluationMethod or
50+
m instanceof MvelCompiledScriptEvaluationMethod
4251
) and
4352
(ma = asExpr() or ma.getQualifier() = asExpr())
4453
)
@@ -99,6 +108,30 @@ predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node nod
99108
)
100109
}
101110

111+
/**
112+
* Holds if `node1` to `node2` is a dataflow step that compiles a script via `MvelScriptEngine`,
113+
* i.e. `engine.compile(tainted)` or `engine.compiledScript(tainted)`.
114+
*/
115+
predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
116+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
117+
m instanceof MvelScriptEngineCompilationnMethod and
118+
(ma = node2.asExpr() or ma.getQualifier() = node2.asExpr()) and
119+
ma.getArgument(0) = node1.asExpr()
120+
)
121+
}
122+
123+
/**
124+
* Holds if `node1` to `node2` is a dataflow step creates `MvelCompiledScript`,
125+
* i.e. `new MvelCompiledScript(engine, tainted)`.
126+
*/
127+
predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) {
128+
exists(ConstructorCall cc |
129+
cc.getConstructedType() instanceof MvelCompiledScript and
130+
(cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
131+
cc.getArgument(1) = node1.asExpr()
132+
)
133+
}
134+
102135
/**
103136
* Methods in the MVEL class that evaluate a MVEL expression.
104137
*/
@@ -131,7 +164,7 @@ class MvelCompileExpressionMethod extends Method {
131164
}
132165

133166
/**
134-
* Methods in `ExecutableStatement` that trigger evaluating a MVEL expression.
167+
* Methods in `ExecutableStatement` that evaluate a MVEL expression.
135168
*/
136169
class ExecutableStatementEvaluationMethod extends Method {
137170
ExecutableStatementEvaluationMethod() {
@@ -141,7 +174,7 @@ class ExecutableStatementEvaluationMethod extends Method {
141174
}
142175

143176
/**
144-
* Methods in `CompiledExpression` that trigger evaluating a MVEL expression.
177+
* Methods in `CompiledExpression` that evaluate a MVEL expression.
145178
*/
146179
class CompiledExpressionEvaluationMethod extends Method {
147180
CompiledExpressionEvaluationMethod() {
@@ -151,7 +184,7 @@ class CompiledExpressionEvaluationMethod extends Method {
151184
}
152185

153186
/**
154-
* Methods in `CompiledAccExpression` that trigger evaluating a MVEL expression.
187+
* Methods in `CompiledAccExpression` that evaluate a MVEL expression.
155188
*/
156189
class CompiledAccExpressionEvaluationMethod extends Method {
157190
CompiledAccExpressionEvaluationMethod() {
@@ -161,7 +194,7 @@ class CompiledAccExpressionEvaluationMethod extends Method {
161194
}
162195

163196
/**
164-
* Methods in `Accessor` that trigger evaluating a MVEL expression.
197+
* Methods in `Accessor` that evaluate a MVEL expression.
165198
*/
166199
class AccessorEvaluationMethod extends Method {
167200
AccessorEvaluationMethod() {
@@ -170,6 +203,46 @@ class AccessorEvaluationMethod extends Method {
170203
}
171204
}
172205

206+
/**
207+
* Methods in `MvelScriptEngine` that evaluate a MVEL expression.
208+
*/
209+
class MvelScriptEngineEvaluationMethod extends Method {
210+
MvelScriptEngineEvaluationMethod() {
211+
getDeclaringType() instanceof MvelScriptEngine and
212+
(hasName("eval") or hasName("evaluate"))
213+
}
214+
}
215+
216+
/**
217+
* Methods in `MvelScriptEngine` that compile a MVEL expression.
218+
*/
219+
class MvelScriptEngineCompilationnMethod extends Method {
220+
MvelScriptEngineCompilationnMethod() {
221+
getDeclaringType() instanceof MvelScriptEngine and
222+
(hasName("compile") or hasName("compiledScript"))
223+
}
224+
}
225+
226+
/**
227+
* Methods in `CompiledScript` that evaluate a MVEL expression.
228+
*/
229+
class CompiledScriptEvaluationMethod extends Method {
230+
CompiledScriptEvaluationMethod() {
231+
getDeclaringType() instanceof CompiledScript and
232+
hasName("eval")
233+
}
234+
}
235+
236+
/**
237+
* Methods in `MvelCompiledScript` that evaluate a MVEL expression.
238+
*/
239+
class MvelCompiledScriptEvaluationMethod extends Method {
240+
MvelCompiledScriptEvaluationMethod() {
241+
getDeclaringType() instanceof MvelCompiledScript and
242+
hasName("eval")
243+
}
244+
}
245+
173246
class MVEL extends RefType {
174247
MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
175248
}
@@ -193,3 +266,15 @@ class CompiledAccExpression extends RefType {
193266
class Accessor extends RefType {
194267
Accessor() { hasQualifiedName("org.mvel2.compiler", "Accessor") }
195268
}
269+
270+
class CompiledScript extends RefType {
271+
CompiledScript() { hasQualifiedName("javax.script", "CompiledScript") }
272+
}
273+
274+
class MvelScriptEngine extends RefType {
275+
MvelScriptEngine() { hasQualifiedName("org.mvel2.jsr223", "MvelScriptEngine") }
276+
}
277+
278+
class MvelCompiledScript extends RefType {
279+
MvelCompiledScript() { hasQualifiedName("org.mvel2.jsr223", "MvelCompiledScript") }
280+
}

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

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,37 @@
11
edges
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:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement |
6-
| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression |
7-
| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression |
2+
| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input |
3+
| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression |
4+
| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement |
5+
| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement |
6+
| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression |
7+
| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression |
8+
| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript |
9+
| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script |
10+
| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script |
811
nodes
9-
| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
10-
| MvelInjection.java:20:17:20:21 | input | semmle.label | input |
11-
| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
12-
| MvelInjection.java:30:30:30:39 | expression | semmle.label | expression |
13-
| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
14-
| MvelInjection.java:41:7:41:15 | statement | semmle.label | statement |
15-
| MvelInjection.java:42:7:42:15 | statement | semmle.label | statement |
16-
| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
17-
| MvelInjection.java:53:7:53:16 | expression | semmle.label | expression |
18-
| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
19-
| MvelInjection.java:63:7:63:16 | expression | semmle.label | expression |
12+
| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
13+
| MvelInjection.java:24:17:24:21 | input | semmle.label | input |
14+
| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
15+
| MvelInjection.java:34:30:34:39 | expression | semmle.label | expression |
16+
| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
17+
| MvelInjection.java:45:7:45:15 | statement | semmle.label | statement |
18+
| MvelInjection.java:46:7:46:15 | statement | semmle.label | statement |
19+
| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
20+
| MvelInjection.java:57:7:57:16 | expression | semmle.label | expression |
21+
| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
22+
| MvelInjection.java:67:7:67:16 | expression | semmle.label | expression |
23+
| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
24+
| MvelInjection.java:80:5:80:18 | compiledScript | semmle.label | compiledScript |
25+
| MvelInjection.java:83:21:83:26 | script | semmle.label | script |
26+
| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
27+
| MvelInjection.java:97:5:97:10 | script | semmle.label | script |
2028
#select
21-
| 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 |
22-
| 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 |
23-
| 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 |
24-
| MvelInjection.java:42:7:42:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
25-
| MvelInjection.java:53:7:53:16 | expression | MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression | MVEL injection from $@. | MvelInjection.java:47:27:47:49 | getInputStream(...) | this user input |
26-
| MvelInjection.java:63:7:63:16 | expression | MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression | MVEL injection from $@. | MvelInjection.java:58:27:58:49 | getInputStream(...) | this user input |
29+
| MvelInjection.java:24:17:24:21 | input | MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input | MVEL injection from $@. | MvelInjection.java:20:27:20:49 | getInputStream(...) | this user input |
30+
| MvelInjection.java:34:30:34:39 | expression | MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression | MVEL injection from $@. | MvelInjection.java:29:27:29:49 | getInputStream(...) | this user input |
31+
| MvelInjection.java:45:7:45:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
32+
| MvelInjection.java:46:7:46:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
33+
| MvelInjection.java:57:7:57:16 | expression | MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression | MVEL injection from $@. | MvelInjection.java:51:27:51:49 | getInputStream(...) | this user input |
34+
| MvelInjection.java:67:7:67:16 | expression | MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression | MVEL injection from $@. | MvelInjection.java:62:27:62:49 | getInputStream(...) | this user input |
35+
| MvelInjection.java:80:5:80:18 | compiledScript | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
36+
| MvelInjection.java:83:21:83:26 | script | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
37+
| MvelInjection.java:97:5:97:10 | script | MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script | MVEL injection from $@. | MvelInjection.java:87:22:87:44 | getInputStream(...) | this user input |

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22
import java.io.InputStream;
33
import java.io.Serializable;
44
import java.net.Socket;
5+
import javax.script.CompiledScript;
6+
import javax.script.SimpleScriptContext;
57
import org.mvel2.MVEL;
68
import org.mvel2.ParserContext;
79
import org.mvel2.compiler.CompiledAccExpression;
810
import org.mvel2.compiler.CompiledExpression;
911
import org.mvel2.compiler.ExecutableStatement;
1012
import org.mvel2.compiler.ExpressionCompiler;
1113
import org.mvel2.integration.impl.ImmutableDefaultFactory;
14+
import org.mvel2.jsr223.MvelCompiledScript;
15+
import org.mvel2.jsr223.MvelScriptEngine;
1216

1317
public class MvelInjection {
1418

@@ -63,4 +67,33 @@ public static void testCompiledAccExpressionGetValue(Socket socket) throws IOExc
6367
expression.getValue(new Object(), new ImmutableDefaultFactory());
6468
}
6569
}
70+
71+
public static void testMvelScriptEngineCompileAndEvaluate(Socket socket) throws Exception {
72+
InputStream in = socket.getInputStream();
73+
74+
byte[] bytes = new byte[1024];
75+
int n = in.read(bytes);
76+
String input = new String(bytes, 0, n);
77+
78+
MvelScriptEngine engine = new MvelScriptEngine();
79+
CompiledScript compiledScript = engine.compile(input);
80+
compiledScript.eval();
81+
82+
Serializable script = engine.compiledScript(input);
83+
engine.evaluate(script, new SimpleScriptContext());
84+
}
85+
86+
public static void testMvelCompiledScriptCompileAndEvaluate(Socket socket) throws Exception {
87+
InputStream in = socket.getInputStream();
88+
89+
byte[] bytes = new byte[1024];
90+
int n = in.read(bytes);
91+
String input = new String(bytes, 0, n);
92+
93+
MvelScriptEngine engine = new MvelScriptEngine();
94+
ExpressionCompiler compiler = new ExpressionCompiler(input);
95+
ExecutableStatement statement = compiler.compile();
96+
MvelCompiledScript script = new MvelCompiledScript(engine, statement);
97+
script.eval(new SimpleScriptContext());
98+
}
6699
}

‎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:${testdir}/../../../../stubs/mvel2-2.4.7
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api

‎java/ql/test/stubs/jsr223-api/javax/script/CompiledScript.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package javax.script;
2+
3+
public class CompiledScript {
4+
public Object eval() throws ScriptException { return null; }
5+
}

‎java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package javax.script;
2+
3+
public interface ScriptContext {}

‎java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package javax.script;
2+
3+
public class ScriptException extends Exception {}

‎java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package javax.script;
2+
3+
public class SimpleScriptContext implements ScriptContext {}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
package org.mvel2.compiler;
22

3+
import java.io.Serializable;
34
import org.mvel2.integration.VariableResolverFactory;
45

5-
public interface ExecutableStatement extends Accessor {
6+
public interface ExecutableStatement extends Accessor, Serializable {
67
public Object getValue(Object staticContext, VariableResolverFactory factory);
78
}

‎java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package org.mvel2.jsr223;
2+
3+
import java.io.Serializable;
4+
import javax.script.CompiledScript;
5+
import javax.script.ScriptContext;
6+
import javax.script.ScriptException;
7+
8+
public class MvelCompiledScript extends CompiledScript {
9+
public MvelCompiledScript(MvelScriptEngine engine, Serializable compiledScript) {}
10+
public Object eval(ScriptContext context) throws ScriptException { return null; }
11+
}

0 commit comments

Comments
 (0)