Skip to content

Commit 43a07bb

Browse files
artem-smotrakovartem-smotrakov
committed
Better sink in SandboxedJexlFlowConfig
1 parent 042c0b0 commit 43a07bb

File tree

3 files changed

+51
-45
lines changed

3 files changed

+51
-45
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* @name Expression language injection (Jexl)
3-
* @description Evaluation of a user-controlled Jexl expression
2+
* @name Expression language injection (JEXL)
3+
* @description Evaluation of a user-controlled JEXL expression
44
* may lead to arbitrary code execution.
55
* @kind path-problem
66
* @problem.severity error
@@ -16,4 +16,4 @@ import DataFlow::PathGraph
1616

1717
from DataFlow::PathNode source, DataFlow::PathNode sink, JexlInjectionConfig conf
1818
where conf.hasFlowPath(source, sink)
19-
select sink.getNode(), source, sink, "Jexl injection from $@.", source.getNode(), "this user input"
19+
select sink.getNode(), source, sink, "JEXL injection from $@.", source.getNode(), "this user input"

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

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ import semmle.code.java.dataflow.TaintTracking
44

55
/**
66
* A taint-tracking configuration for unsafe user input
7-
* that is used to construct and evaluate a Jexl expression.
8-
* It supports both Jexl2 and Jexl3.
7+
* that is used to construct and evaluate a JEXL expression.
8+
* It supports both JEXL 2 and 3.
99
*/
1010
class JexlInjectionConfig extends TaintTracking::Configuration {
1111
JexlInjectionConfig() { this = "JexlInjectionConfig" }
1212

1313
override predicate isSource(DataFlow::Node source) {
1414
source instanceof TaintedSpringRequestBody or
15-
source instanceof RemoteFlowSource
15+
source instanceof RemoteFlowSource or
16+
source instanceof LocalUserInput
1617
}
1718

1819
override predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
@@ -35,9 +36,9 @@ private class TaintedSpringRequestBody extends DataFlow::Node {
3536

3637
/**
3738
* A sink for Expresssion Language injection vulnerabilities via Jexl,
38-
* i.e. method calls that run evaluation of a Jexl expression.
39+
* i.e. method calls that run evaluation of a JEXL expression.
3940
*
40-
* Creating a `Callable` from a tainted Jexl expression or script is considered as a sink
41+
* Creating a `Callable` from a tainted JEXL expression or script is considered as a sink
4142
* although the tainted expression is not executed at this point.
4243
* Here we assume that it will get executed at some point,
4344
* maybe stored in an object field and then reached by a different flow.
@@ -60,7 +61,7 @@ private class JexlEvaluationSink extends DataFlow::ExprNode {
6061

6162
/**
6263
* Defines method calls that propagate tainted data via one of the methods
63-
* from Jexl library.
64+
* from JEXL library.
6465
*/
6566
private class TaintPropagatingJexlMethodCall extends MethodAccess {
6667
Expr taintFromExpr;
@@ -97,24 +98,29 @@ private class TaintPropagatingJexlMethodCall extends MethodAccess {
9798
}
9899

99100
/**
100-
* Holds if `expr` is a Jexl engine that is not configured with a sandbox.
101+
* Holds if `expr` is a JEXL engine that is not configured with a sandbox.
101102
*/
102103
private predicate isUnsafeEngine(Expr expr) {
103104
not exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
104105
}
105106

106107
/**
107-
* A configuration for a tracking sandboxed Jexl engines.
108+
* A configuration for a tracking sandboxed JEXL engines.
108109
*/
109110
private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
110111
SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
111112

112113
override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
113114

114115
override predicate isSink(DataFlow::Node node) {
115-
node.asExpr().getType() instanceof JexlEngine or
116-
node.asExpr().getType() instanceof JxltEngine or
117-
node.asExpr().getType() instanceof UnifiedJexl
116+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
117+
(
118+
m instanceof CreateJexlScriptMethod or
119+
m instanceof CreateJexlExpressionMethod or
120+
m instanceof CreateJexlTemplateMethod
121+
) and
122+
ma.getQualifier() = node.asExpr()
123+
)
118124
}
119125

120126
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
@@ -123,7 +129,7 @@ private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
123129
}
124130

125131
/**
126-
* Defines a data flow source for Jexl engines configured with a sandbox.
132+
* Defines a data flow source for JEXL engines configured with a sandbox.
127133
*/
128134
private class SandboxedJexlSource extends DataFlow::ExprNode {
129135
SandboxedJexlSource() {
@@ -143,7 +149,7 @@ private class SandboxedJexlSource extends DataFlow::ExprNode {
143149
}
144150

145151
/**
146-
* Holds if `fromNode` to `toNode` is a dataflow step that creates one of the Jexl engines.
152+
* Holds if `fromNode` to `toNode` is a dataflow step that creates one of the JEXL engines.
147153
*/
148154
private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNode) {
149155
exists(MethodAccess ma, Method m | m = ma.getMethod() |
@@ -162,7 +168,7 @@ private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNo
162168

163169
/**
164170
* Holds if `fromNode` to `toNode` is a dataflow step that returns data from
165-
* a tainted bean by calling one of its getters.
171+
* a bean by calling one of its getters.
166172
*/
167173
private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node toNode) {
168174
exists(MethodAccess ma, Method m | ma.getMethod() = m |
@@ -173,7 +179,7 @@ private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node to
173179
}
174180

175181
/**
176-
* Method in the `JexlEngine` class that get or set a property with a Jexl expression.
182+
* A methods in the `JexlEngine` class that gets or sets a property with a JEXL expression.
177183
*/
178184
private class JexlEngineGetSetPropertyMethod extends Method {
179185
JexlEngineGetSetPropertyMethod() {
@@ -183,7 +189,7 @@ private class JexlEngineGetSetPropertyMethod extends Method {
183189
}
184190

185191
/**
186-
* Defines methods that triggers direct evaluation of Jexl expressions.
192+
* A method that triggers direct evaluation of JEXL expressions.
187193
*/
188194
private class DirectJexlEvaluationMethod extends Method {
189195
DirectJexlEvaluationMethod() {
@@ -202,14 +208,14 @@ private class DirectJexlEvaluationMethod extends Method {
202208
}
203209

204210
/**
205-
* Defines methods that create a Jexl script.
211+
* A method that creates a JEXL script.
206212
*/
207213
private class CreateJexlScriptMethod extends Method {
208214
CreateJexlScriptMethod() { getDeclaringType() instanceof JexlEngine and hasName("createScript") }
209215
}
210216

211217
/**
212-
* Defines methods that creates a `Callable` for a Jexl expression or script.
218+
* A method that creates a `Callable` for a JEXL expression or script.
213219
*/
214220
private class CreateJexlCallableMethod extends Method {
215221
CreateJexlCallableMethod() {
@@ -219,7 +225,7 @@ private class CreateJexlCallableMethod extends Method {
219225
}
220226

221227
/**
222-
* Defines methods that create a Jexl template.
228+
* A method that creates a JEXL template.
223229
*/
224230
private class CreateJexlTemplateMethod extends Method {
225231
CreateJexlTemplateMethod() {
@@ -229,7 +235,7 @@ private class CreateJexlTemplateMethod extends Method {
229235
}
230236

231237
/**
232-
* Defines methods that create a Jexl expression.
238+
* A method that creates a JEXL expression.
233239
*/
234240
private class CreateJexlExpressionMethod extends Method {
235241
CreateJexlExpressionMethod() {

‎java/ql/test/experimental/query-tests/security/CWE-094/JexlInjection.expected

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -171,25 +171,25 @@ nodes
171171
| Jexl3Injection.java:161:13:161:52 | customRequest : CustomRequest | semmle.label | customRequest : CustomRequest |
172172
| Jexl3Injection.java:164:27:164:30 | expr : String | semmle.label | expr : String |
173173
#select
174-
| Jexl2Injection.java:14:9:14:9 | e | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:14:9:14:9 | e | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
175-
| Jexl2Injection.java:22:9:22:9 | e | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:22:9:22:9 | e | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
176-
| Jexl2Injection.java:29:9:29:14 | script | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:29:9:29:14 | script | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
177-
| Jexl2Injection.java:38:13:38:18 | script | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:38:13:38:18 | script | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
178-
| Jexl2Injection.java:46:40:46:47 | jexlExpr | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:46:40:46:47 | jexlExpr | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
179-
| Jexl2Injection.java:51:40:51:47 | jexlExpr | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:51:40:51:47 | jexlExpr | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
180-
| Jexl2Injection.java:57:9:57:35 | parse(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:57:9:57:35 | parse(...) | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
181-
| Jexl2Injection.java:63:9:63:35 | parse(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:63:9:63:35 | parse(...) | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
182-
| Jexl2Injection.java:69:9:69:44 | createTemplate(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:69:9:69:44 | createTemplate(...) | Jexl injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
183-
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:19:9:19:9 | e | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
184-
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:143:13:143:37 | expr : String | Jexl3Injection.java:19:9:19:9 | e | Jexl injection from $@. | Jexl3Injection.java:143:13:143:37 | expr | this user input |
185-
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:151:13:151:34 | data : Data | Jexl3Injection.java:19:9:19:9 | e | Jexl injection from $@. | Jexl3Injection.java:151:13:151:34 | data | this user input |
186-
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:161:13:161:52 | customRequest : CustomRequest | Jexl3Injection.java:19:9:19:9 | e | Jexl injection from $@. | Jexl3Injection.java:161:13:161:52 | customRequest | this user input |
187-
| Jexl3Injection.java:26:9:26:9 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:26:9:26:9 | e | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
188-
| Jexl3Injection.java:33:9:33:14 | script | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:33:9:33:14 | script | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
189-
| Jexl3Injection.java:42:13:42:18 | script | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:42:13:42:18 | script | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
190-
| Jexl3Injection.java:50:40:50:47 | jexlExpr | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:50:40:50:47 | jexlExpr | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
191-
| Jexl3Injection.java:55:40:55:47 | jexlExpr | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:55:40:55:47 | jexlExpr | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
192-
| Jexl3Injection.java:61:9:61:39 | createExpression(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:61:9:61:39 | createExpression(...) | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
193-
| Jexl3Injection.java:67:9:67:39 | createExpression(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:67:9:67:39 | createExpression(...) | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
194-
| Jexl3Injection.java:73:9:73:37 | createTemplate(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:73:9:73:37 | createTemplate(...) | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
195-
| Jexl3Injection.java:82:13:82:13 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:82:13:82:13 | e | Jexl injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
174+
| Jexl2Injection.java:14:9:14:9 | e | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:14:9:14:9 | e | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
175+
| Jexl2Injection.java:22:9:22:9 | e | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:22:9:22:9 | e | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
176+
| Jexl2Injection.java:29:9:29:14 | script | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:29:9:29:14 | script | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
177+
| Jexl2Injection.java:38:13:38:18 | script | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:38:13:38:18 | script | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
178+
| Jexl2Injection.java:46:40:46:47 | jexlExpr | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:46:40:46:47 | jexlExpr | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
179+
| Jexl2Injection.java:51:40:51:47 | jexlExpr | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:51:40:51:47 | jexlExpr | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
180+
| Jexl2Injection.java:57:9:57:35 | parse(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:57:9:57:35 | parse(...) | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
181+
| Jexl2Injection.java:63:9:63:35 | parse(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:63:9:63:35 | parse(...) | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
182+
| Jexl2Injection.java:69:9:69:44 | createTemplate(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:69:9:69:44 | createTemplate(...) | JEXL injection from $@. | Jexl2Injection.java:76:25:76:47 | getInputStream(...) | this user input |
183+
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:19:9:19:9 | e | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
184+
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:143:13:143:37 | expr : String | Jexl3Injection.java:19:9:19:9 | e | JEXL injection from $@. | Jexl3Injection.java:143:13:143:37 | expr | this user input |
185+
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:151:13:151:34 | data : Data | Jexl3Injection.java:19:9:19:9 | e | JEXL injection from $@. | Jexl3Injection.java:151:13:151:34 | data | this user input |
186+
| Jexl3Injection.java:19:9:19:9 | e | Jexl3Injection.java:161:13:161:52 | customRequest : CustomRequest | Jexl3Injection.java:19:9:19:9 | e | JEXL injection from $@. | Jexl3Injection.java:161:13:161:52 | customRequest | this user input |
187+
| Jexl3Injection.java:26:9:26:9 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:26:9:26:9 | e | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
188+
| Jexl3Injection.java:33:9:33:14 | script | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:33:9:33:14 | script | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
189+
| Jexl3Injection.java:42:13:42:18 | script | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:42:13:42:18 | script | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
190+
| Jexl3Injection.java:50:40:50:47 | jexlExpr | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:50:40:50:47 | jexlExpr | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
191+
| Jexl3Injection.java:55:40:55:47 | jexlExpr | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:55:40:55:47 | jexlExpr | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
192+
| Jexl3Injection.java:61:9:61:39 | createExpression(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:61:9:61:39 | createExpression(...) | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
193+
| Jexl3Injection.java:67:9:67:39 | createExpression(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:67:9:67:39 | createExpression(...) | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
194+
| Jexl3Injection.java:73:9:73:37 | createTemplate(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:73:9:73:37 | createTemplate(...) | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |
195+
| Jexl3Injection.java:82:13:82:13 | e | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:82:13:82:13 | e | JEXL injection from $@. | Jexl3Injection.java:92:25:92:47 | getInputStream(...) | this user input |

0 commit comments

Comments
 (0)