Skip to content

Commit 53e04d0

Browse files
atorralbaatorralba
committed
Refactor to CSV sink model
1 parent ecd40e5 commit 53e04d0

File tree

2 files changed

+123
-66
lines changed

2 files changed

+123
-66
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@
1313
import java
1414
import JexlInjectionLib
1515
import DataFlow::PathGraph
16+
import FlowUtils
17+
18+
/**
19+
* A taint-tracking configuration for unsafe user input
20+
* that is used to construct and evaluate a JEXL expression.
21+
* It supports both JEXL 2 and 3.
22+
*/
23+
class JexlInjectionConfig extends TaintTracking::Configuration {
24+
JexlInjectionConfig() { this = "JexlInjectionConfig" }
25+
26+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
27+
28+
override predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
29+
30+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
31+
any(JexlInjectionAdditionalTaintStep c).step(node1, node2) or
32+
hasGetterFlow(node1, node2)
33+
}
34+
}
1635

1736
from DataFlow::PathNode source, DataFlow::PathNode sink, JexlInjectionConfig conf
1837
where conf.hasFlowPath(source, sink)

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

Lines changed: 104 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,87 +1,125 @@
11
import java
2-
import FlowUtils
3-
import semmle.code.java.dataflow.FlowSources
42
import semmle.code.java.dataflow.TaintTracking
3+
private import semmle.code.java.dataflow.ExternalFlow
54

65
/**
7-
* A taint-tracking configuration for unsafe user input
8-
* that is used to construct and evaluate a JEXL expression.
9-
* It supports both JEXL 2 and 3.
6+
* A sink for Expresssion Language injection vulnerabilities via Jexl,
7+
* i.e. method calls that run evaluation of a JEXL expression.
108
*/
11-
class JexlInjectionConfig extends TaintTracking::Configuration {
12-
JexlInjectionConfig() { this = "JexlInjectionConfig" }
9+
abstract class JexlEvaluationSink extends DataFlow::ExprNode { }
1310

14-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
15-
16-
override predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
11+
/** Default sink for JXEL injection vulnerabilities. */
12+
private class DefaultJexlEvaluationSink extends JexlEvaluationSink {
13+
DefaultJexlEvaluationSink() { sinkNode(this, "jexl") }
14+
}
1715

18-
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
19-
any(TaintPropagatingJexlMethodCall c).taintFlow(fromNode, toNode) or
20-
hasGetterFlow(fromNode, toNode)
16+
private class DefaultJexlInjectionSinkModel extends SinkModelCsv {
17+
override predicate row(string row) {
18+
row =
19+
[
20+
// JEXL2
21+
"org.apache.commons.jexl2;JexlEngine;false;getProperty;(JexlContext,Object,String);;Argument[2];jexl",
22+
"org.apache.commons.jexl2;JexlEngine;false;getProperty;(Object,String);;Argument[1];jexl",
23+
"org.apache.commons.jexl2;JexlEngine;false;setProperty;(JexlContext,Object,String,Object);;Argument[2];jexl",
24+
"org.apache.commons.jexl2;JexlEngine;false;setProperty;(Object,String,Object);;Argument[1];jexl",
25+
"org.apache.commons.jexl2;Expression;false;evaluate;;;Argument[-1];jexl",
26+
"org.apache.commons.jexl2;Expression;false;callable;;;Argument[-1];jexl",
27+
"org.apache.commons.jexl2;JexlExpression;false;evaluate;;;Argument[-1];jexl",
28+
"org.apache.commons.jexl2;JexlExpression;false;callable;;;Argument[-1];jexl",
29+
"org.apache.commons.jexl2;Script;false;execute;;;Argument[-1];jexl",
30+
"org.apache.commons.jexl2;Script;false;callable;;;Argument[-1];jexl",
31+
"org.apache.commons.jexl2;JexlScript;false;execute;;;Argument[-1];jexl",
32+
"org.apache.commons.jexl2;JexlScript;false;callable;;;Argument[-1];jexl",
33+
"org.apache.commons.jexl2;JxltEngine$Expression;false;evaluate;;;Argument[-1];jexl",
34+
"org.apache.commons.jexl2;JxltEngine$Expression;false;prepare;;;Argument[-1];jexl",
35+
"org.apache.commons.jexl2;JxltEngine$Template;false;evaluate;;;Argument[-1];jexl",
36+
"org.apache.commons.jexl2;UnifiedJEXL$Expression;false;evaluate;;;Argument[-1];jexl",
37+
"org.apache.commons.jexl2;UnifiedJEXL$Expression;false;prepare;;;Argument[-1];jexl",
38+
"org.apache.commons.jexl2;UnifiedJEXL$Template;false;evaluate;;;Argument[-1];jexl",
39+
// JEXL3
40+
"org.apache.commons.jexl3;JexlEngine;false;getProperty;(JexlContext,Object,String);;Argument[2];jexl",
41+
"org.apache.commons.jexl3;JexlEngine;false;getProperty;(Object,String);;Argument[1];jexl",
42+
"org.apache.commons.jexl3;JexlEngine;false;setProperty;(JexlContext,Object,String);;Argument[2];jexl",
43+
"org.apache.commons.jexl3;JexlEngine;false;setProperty;(Object,String,Object);;Argument[1];jexl",
44+
"org.apache.commons.jexl3;Expression;false;evaluate;;;Argument[-1];jexl",
45+
"org.apache.commons.jexl3;Expression;false;callable;;;Argument[-1];jexl",
46+
"org.apache.commons.jexl3;JexlExpression;false;evaluate;;;Argument[-1];jexl",
47+
"org.apache.commons.jexl3;JexlExpression;false;callable;;;Argument[-1];jexl",
48+
"org.apache.commons.jexl3;Script;false;execute;;;Argument[-1];jexl",
49+
"org.apache.commons.jexl3;Script;false;callable;;;Argument[-1];jexl",
50+
"org.apache.commons.jexl3;JexlScript;false;execute;;;Argument[-1];jexl",
51+
"org.apache.commons.jexl3;JexlScript;false;callable;;;Argument[-1];jexl",
52+
"org.apache.commons.jexl3;JxltEngine$Expression;false;evaluate;;;Argument[-1];jexl",
53+
"org.apache.commons.jexl3;JxltEngine$Expression;false;prepare;;;Argument[-1];jexl",
54+
"org.apache.commons.jexl3;JxltEngine$Template;false;evaluate;;;Argument[-1];jexl",
55+
"org.apache.commons.jexl3;UnifiedJEXL$Expression;false;evaluate;;;Argument[-1];jexl",
56+
"org.apache.commons.jexl3;UnifiedJEXL$Expression;false;prepare;;;Argument[-1];jexl",
57+
"org.apache.commons.jexl3;UnifiedJEXL$Template;false;evaluate;;;Argument[-1];jexl"
58+
]
2159
}
2260
}
2361

2462
/**
25-
* A sink for Expresssion Language injection vulnerabilities via Jexl,
26-
* i.e. method calls that run evaluation of a JEXL expression.
63+
* A unit class for adding additional taint steps.
2764
*
28-
* Creating a `Callable` from a tainted JEXL expression or script is considered as a sink
29-
* although the tainted expression is not executed at this point.
30-
* Here we assume that it will get executed at some point,
31-
* maybe stored in an object field and then reached by a different flow.
65+
* Extend this class to add additional taint steps that should apply to the `JexlInjectionFlowConfig`.
3266
*/
33-
private class JexlEvaluationSink extends DataFlow::ExprNode {
34-
JexlEvaluationSink() {
35-
exists(MethodAccess ma, Method m, Expr taintFrom |
36-
ma.getMethod() = m and taintFrom = this.asExpr()
37-
|
38-
m instanceof DirectJexlEvaluationMethod and ma.getQualifier() = taintFrom
39-
or
40-
m instanceof CreateJexlCallableMethod and ma.getQualifier() = taintFrom
41-
or
42-
m instanceof JexlEngineGetSetPropertyMethod and
43-
taintFrom.getType() instanceof TypeString and
44-
ma.getAnArgument() = taintFrom
45-
)
67+
abstract class JexlInjectionAdditionalTaintStep extends Unit {
68+
/**
69+
* Holds if the step from `node1` to `node2` should be considered a taint
70+
* step for the `JexlInjectionConfig` configuration.
71+
*/
72+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
73+
}
74+
75+
/** A set of additional taint steps to consider when taint tracking JXEL related data flows. */
76+
private class DefaultJexlInjectionAdditionalTaintStep extends JexlInjectionAdditionalTaintStep {
77+
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
78+
createJexlScriptStep(node1, node2) or
79+
createJexlExpressionStep(node1, node2) or
80+
createJexlTemplateStep(node1, node2)
4681
}
4782
}
4883

4984
/**
50-
* Defines method calls that propagate tainted data via one of the methods
51-
* from JEXL library.
85+
* Holds if `n1` to `n2` is a dataflow step that creates a JEXL script using an unsafe engine
86+
* i.e. `tainted.createScript(jexlExpr)`.
5287
*/
53-
private class TaintPropagatingJexlMethodCall extends MethodAccess {
54-
Expr taintFromExpr;
55-
56-
TaintPropagatingJexlMethodCall() {
57-
exists(Method m, RefType taintType |
58-
this.getMethod() = m and
59-
taintType = taintFromExpr.getType()
60-
|
61-
isUnsafeEngine(this.getQualifier()) and
62-
(
63-
m instanceof CreateJexlScriptMethod and
64-
taintFromExpr = this.getArgument(0) and
65-
taintType instanceof TypeString
66-
or
67-
m instanceof CreateJexlExpressionMethod and
68-
taintFromExpr = this.getAnArgument() and
69-
taintType instanceof TypeString
70-
or
71-
m instanceof CreateJexlTemplateMethod and
72-
(taintType instanceof TypeString or taintType instanceof Reader) and
73-
taintFromExpr = this.getArgument([0, 1])
74-
)
75-
)
76-
}
88+
private predicate createJexlScriptStep(DataFlow::Node n1, DataFlow::Node n2) {
89+
exists(MethodAccess ma, Method m | m = ma.getMethod() and n2.asExpr() = ma |
90+
isUnsafeEngine(ma.getQualifier()) and
91+
m instanceof CreateJexlScriptMethod and
92+
n1.asExpr() = ma.getArgument(0) and
93+
n1.asExpr().getType() instanceof TypeString
94+
)
95+
}
7796

78-
/**
79-
* Holds if `fromNode` to `toNode` is a dataflow step that propagates
80-
* tainted data.
81-
*/
82-
predicate taintFlow(DataFlow::Node fromNode, DataFlow::Node toNode) {
83-
fromNode.asExpr() = taintFromExpr and toNode.asExpr() = this
84-
}
97+
/**
98+
* Holds if `n1` to `n2` is a dataflow step that creates a JEXL expression using an unsafe engine
99+
* i.e. `tainted.createExpression(jexlExpr)`.
100+
*/
101+
private predicate createJexlExpressionStep(DataFlow::Node n1, DataFlow::Node n2) {
102+
exists(MethodAccess ma, Method m | m = ma.getMethod() and n2.asExpr() = ma |
103+
isUnsafeEngine(ma.getQualifier()) and
104+
m instanceof CreateJexlExpressionMethod and
105+
n1.asExpr() = ma.getAnArgument() and
106+
n1.asExpr().getType() instanceof TypeString
107+
)
108+
}
109+
110+
/**
111+
* Holds if `n1` to `n2` is a dataflow step that creates a JEXL template using an unsafe engine
112+
* i.e. `tainted.createTemplate(jexlExpr)`.
113+
*/
114+
private predicate createJexlTemplateStep(DataFlow::Node n1, DataFlow::Node n2) {
115+
exists(MethodAccess ma, Method m, RefType taintType |
116+
m = ma.getMethod() and n2.asExpr() = ma and taintType = n1.asExpr().getType()
117+
|
118+
isUnsafeEngine(ma.getQualifier()) and
119+
m instanceof CreateJexlTemplateMethod and
120+
n1.asExpr() = ma.getArgument([0, 1]) and
121+
(taintType instanceof TypeString or taintType instanceof Reader)
122+
)
85123
}
86124

87125
/**
@@ -92,7 +130,7 @@ private predicate isUnsafeEngine(Expr expr) {
92130
}
93131

94132
/**
95-
* A configuration for a tracking sandboxed JEXL engines.
133+
* A configuration for tracking sandboxed JEXL engines.
96134
*/
97135
private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
98136
SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }

0 commit comments

Comments
 (0)