Skip to content

Commit 8bf3315

Browse files
committed
Refactor JexlInjection
1 parent 7ee6c06 commit 8bf3315

File tree

3 files changed

+35
-14
lines changed

3 files changed

+35
-14
lines changed

java/ql/lib/semmle/code/java/security/JexlInjectionQuery.qll

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ private class DefaultJexlInjectionAdditionalTaintStep extends JexlInjectionAddit
3939
}
4040

4141
/**
42+
* DEPRECATED: Use `JexlInjectionFlow` instead.
43+
*
4244
* A taint-tracking configuration for unsafe user input
4345
* that is used to construct and evaluate a JEXL expression.
4446
* It supports both JEXL 2 and 3.
4547
*/
46-
class JexlInjectionConfig extends TaintTracking::Configuration {
48+
deprecated class JexlInjectionConfig extends TaintTracking::Configuration {
4749
JexlInjectionConfig() { this = "JexlInjectionConfig" }
4850

4951
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -55,6 +57,27 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
5557
}
5658
}
5759

60+
/**
61+
* A taint-tracking configuration for unsafe user input
62+
* that is used to construct and evaluate a JEXL expression.
63+
* It supports both JEXL 2 and 3.
64+
*/
65+
private module JexlInjectionConfig implements DataFlow::ConfigSig {
66+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
67+
68+
predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
69+
70+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
71+
any(JexlInjectionAdditionalTaintStep c).step(node1, node2)
72+
}
73+
}
74+
75+
/**
76+
* Tracks unsafe user input that is used to construct and evaluate a JEXL expression.
77+
* It supports both JEXL 2 and 3.
78+
*/
79+
module JexlInjectionFlow = TaintTracking::Make<JexlInjectionConfig>;
80+
5881
/**
5982
* Holds if `n1` to `n2` is a dataflow step that creates a JEXL script using an unsafe engine
6083
* by calling `tainted.createScript(jexlExpr)`.
@@ -99,19 +122,15 @@ private predicate createJexlTemplateStep(DataFlow::Node n1, DataFlow::Node n2) {
99122
/**
100123
* Holds if `expr` is a JEXL engine that is configured with a sandbox.
101124
*/
102-
private predicate isSafeEngine(Expr expr) {
103-
exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
104-
}
125+
private predicate isSafeEngine(Expr expr) { SandboxedJexlFlow::hasFlowToExpr(expr) }
105126

106127
/**
107128
* A configuration for tracking sandboxed JEXL engines.
108129
*/
109-
private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
110-
SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
130+
private module SandboxedJexlFlowConfig implements DataFlow::ConfigSig {
131+
predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
111132

112-
override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
113-
114-
override predicate isSink(DataFlow::Node node) {
133+
predicate isSink(DataFlow::Node node) {
115134
exists(MethodAccess ma, Method m |
116135
m instanceof CreateJexlScriptMethod or
117136
m instanceof CreateJexlExpressionMethod or
@@ -121,11 +140,13 @@ private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
121140
)
122141
}
123142

124-
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
143+
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
125144
createJexlEngineStep(fromNode, toNode)
126145
}
127146
}
128147

148+
private module SandboxedJexlFlow = DataFlow::Make<SandboxedJexlFlowConfig>;
149+
129150
/**
130151
* Defines a data flow source for JEXL engines configured with a sandbox.
131152
*/

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313

1414
import java
1515
import semmle.code.java.security.JexlInjectionQuery
16-
import DataFlow::PathGraph
16+
import JexlInjectionFlow::PathGraph
1717

18-
from DataFlow::PathNode source, DataFlow::PathNode sink, JexlInjectionConfig conf
19-
where conf.hasFlowPath(source, sink)
18+
from JexlInjectionFlow::PathNode source, JexlInjectionFlow::PathNode sink
19+
where JexlInjectionFlow::hasFlowPath(source, sink)
2020
select sink.getNode(), source, sink, "JEXL expression depends on a $@.", source.getNode(),
2121
"user-provided value"

java/ql/test/query-tests/security/CWE-094/JexlInjectionTest.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class JexlInjectionTest extends InlineExpectationsTest {
99

1010
override predicate hasActualResult(Location location, string element, string tag, string value) {
1111
tag = "hasJexlInjection" and
12-
exists(DataFlow::Node sink, JexlInjectionConfig conf | conf.hasFlowTo(sink) |
12+
exists(DataFlow::Node sink | JexlInjectionFlow::hasFlowTo(sink) |
1313
sink.getLocation() = location and
1414
element = sink.toString() and
1515
value = ""

0 commit comments

Comments
 (0)