Skip to content

Commit 46faf68

Browse files
atorralbaatorralba
committed
Decouple MvelInjection.qll to reuse the taint tracking configuration
1 parent 5ca8b38 commit 46faf68

File tree

6 files changed

+56
-71
lines changed

6 files changed

+56
-71
lines changed

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

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,9 @@
1111
*/
1212

1313
import java
14-
import semmle.code.java.dataflow.FlowSources
15-
import semmle.code.java.dataflow.TaintTracking
16-
import semmle.code.java.security.MvelInjection
14+
import semmle.code.java.security.MvelInjectionQuery
1715
import DataFlow::PathGraph
1816

19-
/**
20-
* A taint-tracking configuration for unsafe user input
21-
* that is used to construct and evaluate a MVEL expression.
22-
*/
23-
class MvelInjectionFlowConfig extends TaintTracking::Configuration {
24-
MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
25-
26-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
27-
28-
override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
29-
30-
override predicate isSanitizer(DataFlow::Node sanitizer) {
31-
sanitizer instanceof MvelInjectionSanitizer
32-
}
33-
34-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
35-
any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
36-
}
37-
}
38-
3917
from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionFlowConfig conf
4018
where conf.hasFlowPath(source, sink)
4119
select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input"

‎java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ private module Frameworks {
9898
private import semmle.code.java.security.InformationLeak
9999
private import semmle.code.java.security.JexlInjectionSinkModels
100100
private import semmle.code.java.security.LdapInjection
101-
private import semmle.code.java.security.MvelInjection
101+
private import semmle.code.java.security.MvelInjectionSinkModels
102102
private import semmle.code.java.security.XPath
103103
private import semmle.code.java.frameworks.android.SQLite
104104
private import semmle.code.java.frameworks.Jdbc

‎java/ql/src/semmle/code/java/security/MvelInjection.qll renamed to ‎java/ql/src/semmle/code/java/security/MvelInjectionQuery.qll

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import java
44
import semmle.code.java.dataflow.DataFlow
55
import semmle.code.java.dataflow.ExternalFlow
6+
import semmle.code.java.dataflow.FlowSources
7+
import semmle.code.java.dataflow.TaintTracking
68

79
/** A data flow sink for unvalidated user input that is used to construct MVEL expressions. */
810
abstract class MvelEvaluationSink extends DataFlow::Node { }
@@ -23,31 +25,6 @@ class MvelInjectionAdditionalTaintStep extends Unit {
2325
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
2426
}
2527

26-
private class DefaulMvelEvaluationSinkModel extends SinkModelCsv {
27-
override predicate row(string row) {
28-
row =
29-
[
30-
"javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
31-
"org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
32-
"org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
33-
"org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
34-
"org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
35-
"org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
36-
"org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
37-
"org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel",
38-
"org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
39-
"org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
40-
"org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
41-
"org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
42-
"org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
43-
"org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
44-
"org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
45-
"org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
46-
"org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel"
47-
]
48-
}
49-
}
50-
5128
/** Default sink for MVEL injection vulnerabilities. */
5229
private class DefaultMvelEvaluationSink extends MvelEvaluationSink {
5330
DefaultMvelEvaluationSink() { sinkNode(this, "mvel") }
@@ -74,6 +51,26 @@ private class DefaultMvelInjectionAdditionalTaintStep extends MvelInjectionAddit
7451
}
7552
}
7653

54+
/**
55+
* A taint-tracking configuration for unsafe user input
56+
* that is used to construct and evaluate a MVEL expression.
57+
*/
58+
class MvelInjectionFlowConfig extends TaintTracking::Configuration {
59+
MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
60+
61+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
62+
63+
override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
64+
65+
override predicate isSanitizer(DataFlow::Node sanitizer) {
66+
sanitizer instanceof MvelInjectionSanitizer
67+
}
68+
69+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
70+
any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
71+
}
72+
}
73+
7774
/**
7875
* Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
7976
* by callilng `MVEL.compileExpression(tainted)`.

‎java/ql/src/semmle/code/java/security/MvelInjectionSinkModels.qll

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/** Provides sink models relating to MVEL injection vulnerabilities. */
2+
3+
import semmle.code.java.dataflow.ExternalFlow
4+
5+
private class DefaulMvelEvaluationSinkModel extends SinkModelCsv {
6+
override predicate row(string row) {
7+
row =
8+
[
9+
"javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
10+
"org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
11+
"org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
12+
"org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
13+
"org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
14+
"org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
15+
"org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
16+
"org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel",
17+
"org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
18+
"org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
19+
"org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
20+
"org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
21+
"org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
22+
"org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
23+
"org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
24+
"org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
25+
"org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel"
26+
]
27+
}
28+
}

‎java/ql/test/experimental/query-tests/security/CWE-094/options

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
<<<<<<< HEAD
2-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13
3-
=======
41
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../experimental/stubs/jshell
5-
>>>>>>> main

‎java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.ql

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,19 @@
11
import java
22
import semmle.code.java.dataflow.TaintTracking
33
import semmle.code.java.dataflow.FlowSources
4-
import semmle.code.java.security.MvelInjection
4+
import semmle.code.java.security.MvelInjectionQuery
55
import TestUtilities.InlineExpectationsTest
66

7-
class Conf extends TaintTracking::Configuration {
8-
Conf() { this = "test:cwe:mvel-injection" }
9-
10-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
11-
12-
override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
13-
14-
override predicate isSanitizer(DataFlow::Node sanitizer) {
15-
sanitizer instanceof MvelInjectionSanitizer
16-
}
17-
18-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
19-
any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
20-
}
21-
}
22-
237
class HasMvelInjectionTest extends InlineExpectationsTest {
248
HasMvelInjectionTest() { this = "HasMvelInjectionTest" }
259

2610
override string getARelevantTag() { result = "hasMvelInjection" }
2711

2812
override predicate hasActualResult(Location location, string element, string tag, string value) {
2913
tag = "hasMvelInjection" and
30-
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
14+
exists(DataFlow::Node src, DataFlow::Node sink, MvelInjectionFlowConfig conf |
15+
conf.hasFlow(src, sink)
16+
|
3117
sink.getLocation() = location and
3218
element = sink.toString() and
3319
value = ""

0 commit comments

Comments
 (0)