Skip to content

Commit 7543df6

Browse files
artem-smotrakovartem-smotrakov
committed
Callable.call() should not be a sink in JexlInjection.ql
1 parent af0f361 commit 7543df6

File tree

2 files changed

+32
-47
lines changed

2 files changed

+32
-47
lines changed

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

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ private class TaintedSpringRequestBody extends DataFlow::Node {
3636
/**
3737
* A sink for Expresssion Language injection vulnerabilities via Jexl,
3838
* i.e. method calls that run evaluation of a Jexl expression.
39+
*
40+
* Creating a `Callable` from a tainted Jexl expression or script is considered as a sink
41+
* although the tainted expression is not executed at this point.
42+
* Here we assume that it will get executed at some point,
43+
* maybe stored in an object field and then reached by a different flow.
3944
*/
4045
private class JexlEvaluationSink extends DataFlow::ExprNode {
4146
JexlEvaluationSink() {
@@ -44,13 +49,11 @@ private class JexlEvaluationSink extends DataFlow::ExprNode {
4449
|
4550
m instanceof DirectJexlEvaluationMethod and ma.getQualifier() = taintFrom
4651
or
47-
m instanceof CallableCallMethod and ma.getQualifier() = taintFrom
52+
m instanceof CreateJexlCallableMethod and ma.getQualifier() = taintFrom
4853
or
4954
m instanceof JexlEngineGetSetPropertyMethod and
50-
exists(Expr arg, int index | arg = ma.getArgument(index) and index = [1, 2] |
51-
arg.getType() instanceof TypeString and
52-
arg = taintFrom
53-
)
55+
taintFrom.getType() instanceof TypeString and
56+
ma.getAnArgument() = taintFrom
5457
)
5558
}
5659
}
@@ -67,23 +70,20 @@ private class TaintPropagatingJexlMethodCall extends MethodAccess {
6770
this.getMethod() = m and
6871
taintType = taintFromExpr.getType()
6972
|
70-
m instanceof CreateJexlScriptMethod and
71-
taintFromExpr = this.getArgument(0) and
72-
taintType instanceof TypeString and
73-
isUnsafeEngine(this.getQualifier())
74-
or
75-
m instanceof CreateJexlCallableMethod and
76-
taintFromExpr = this.getQualifier()
77-
or
78-
m instanceof CreateJexlExpressionMethod and
79-
taintFromExpr = this.getAnArgument() and
80-
taintType instanceof TypeString and
81-
isUnsafeEngine(this.getQualifier())
82-
or
83-
m instanceof CreateJexlTemplateMethod and
84-
(taintType instanceof TypeString or taintType instanceof Reader) and
85-
taintFromExpr = this.getArgument([0, 1]) and
86-
isUnsafeEngine(this.getQualifier())
73+
isUnsafeEngine(this.getQualifier()) and
74+
(
75+
m instanceof CreateJexlScriptMethod and
76+
taintFromExpr = this.getArgument(0) and
77+
taintType instanceof TypeString
78+
or
79+
m instanceof CreateJexlExpressionMethod and
80+
taintFromExpr = this.getAnArgument() and
81+
taintType instanceof TypeString
82+
or
83+
m instanceof CreateJexlTemplateMethod and
84+
(taintType instanceof TypeString or taintType instanceof Reader) and
85+
taintFromExpr = this.getArgument([0, 1])
86+
)
8787
)
8888
}
8989

@@ -97,7 +97,7 @@ private class TaintPropagatingJexlMethodCall extends MethodAccess {
9797
}
9898

9999
/**
100-
* Holds if `expr` is one of the Jexl engines that is not configured with a sandbox.
100+
* Holds if `expr` is a Jexl engine that is not configured with a sandbox.
101101
*/
102102
private predicate isUnsafeEngine(Expr expr) {
103103
not exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
@@ -182,13 +182,6 @@ private class DirectJexlEvaluationMethod extends Method {
182182
}
183183
}
184184

185-
/**
186-
* A method in the `Callable` class that executes the `Callable`.
187-
*/
188-
private class CallableCallMethod extends Method {
189-
CallableCallMethod() { getDeclaringType() instanceof CallableInterface and hasName("call") }
190-
}
191-
192185
/**
193186
* Defines methods that create a Jexl script.
194187
*/
@@ -272,14 +265,6 @@ private class UnifiedJexlTemplate extends NestedType {
272265
UnifiedJexlTemplate() { getEnclosingType() instanceof UnifiedJexl and hasName("Template") }
273266
}
274267

275-
private class CallableInterface extends RefType {
276-
CallableInterface() {
277-
getSourceDeclaration()
278-
.getASourceSupertype*()
279-
.hasQualifiedName("java.util.concurrent", "Callable")
280-
}
281-
}
282-
283268
private class Reader extends RefType {
284269
Reader() { hasQualifiedName("java.io", "Reader") }
285270
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ edges
22
| Jexl2Injection.java:10:43:10:57 | jexlExpr : String | Jexl2Injection.java:14:9:14:9 | e |
33
| Jexl2Injection.java:17:55:17:69 | jexlExpr : String | Jexl2Injection.java:22:9:22:9 | e |
44
| Jexl2Injection.java:25:39:25:53 | jexlExpr : String | Jexl2Injection.java:29:9:29:14 | script |
5-
| Jexl2Injection.java:32:50:32:64 | jexlExpr : String | Jexl2Injection.java:38:13:38:31 | callable(...) |
5+
| Jexl2Injection.java:32:50:32:64 | jexlExpr : String | Jexl2Injection.java:38:13:38:18 | script |
66
| Jexl2Injection.java:44:57:44:71 | jexlExpr : String | Jexl2Injection.java:46:40:46:47 | jexlExpr |
77
| Jexl2Injection.java:49:57:49:71 | jexlExpr : String | Jexl2Injection.java:51:40:51:47 | jexlExpr |
88
| Jexl2Injection.java:54:73:54:87 | jexlExpr : String | Jexl2Injection.java:57:9:57:35 | parse(...) |
@@ -39,13 +39,13 @@ edges
3939
| Jexl3Injection.java:15:43:15:57 | jexlExpr : String | Jexl3Injection.java:19:9:19:9 | e |
4040
| Jexl3Injection.java:22:55:22:69 | jexlExpr : String | Jexl3Injection.java:26:9:26:9 | e |
4141
| Jexl3Injection.java:29:39:29:53 | jexlExpr : String | Jexl3Injection.java:33:9:33:14 | script |
42-
| Jexl3Injection.java:36:50:36:64 | jexlExpr : String | Jexl3Injection.java:42:13:42:31 | callable(...) |
42+
| Jexl3Injection.java:36:50:36:64 | jexlExpr : String | Jexl3Injection.java:42:13:42:18 | script |
4343
| Jexl3Injection.java:48:57:48:71 | jexlExpr : String | Jexl3Injection.java:50:40:50:47 | jexlExpr |
4444
| Jexl3Injection.java:53:57:53:71 | jexlExpr : String | Jexl3Injection.java:55:40:55:47 | jexlExpr |
4545
| Jexl3Injection.java:58:74:58:88 | jexlExpr : String | Jexl3Injection.java:61:9:61:39 | createExpression(...) |
4646
| Jexl3Injection.java:64:73:64:87 | jexlExpr : String | Jexl3Injection.java:67:9:67:39 | createExpression(...) |
4747
| Jexl3Injection.java:70:72:70:86 | jexlExpr : String | Jexl3Injection.java:73:9:73:37 | createTemplate(...) |
48-
| Jexl3Injection.java:76:54:76:68 | jexlExpr : String | Jexl3Injection.java:82:13:82:26 | callable(...) |
48+
| Jexl3Injection.java:76:54:76:68 | jexlExpr : String | Jexl3Injection.java:82:13:82:13 | e |
4949
| Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:94:31:94:38 | jexlExpr : String |
5050
| Jexl3Injection.java:94:31:94:38 | jexlExpr : String | Jexl3Injection.java:102:24:102:56 | jexlExpr : String |
5151
| Jexl3Injection.java:94:31:94:38 | jexlExpr : String | Jexl3Injection.java:106:24:106:68 | jexlExpr : String |
@@ -91,7 +91,7 @@ nodes
9191
| Jexl2Injection.java:25:39:25:53 | jexlExpr : String | semmle.label | jexlExpr : String |
9292
| Jexl2Injection.java:29:9:29:14 | script | semmle.label | script |
9393
| Jexl2Injection.java:32:50:32:64 | jexlExpr : String | semmle.label | jexlExpr : String |
94-
| Jexl2Injection.java:38:13:38:31 | callable(...) | semmle.label | callable(...) |
94+
| Jexl2Injection.java:38:13:38:18 | script | semmle.label | script |
9595
| Jexl2Injection.java:44:57:44:71 | jexlExpr : String | semmle.label | jexlExpr : String |
9696
| Jexl2Injection.java:46:40:46:47 | jexlExpr | semmle.label | jexlExpr |
9797
| Jexl2Injection.java:49:57:49:71 | jexlExpr : String | semmle.label | jexlExpr : String |
@@ -129,7 +129,7 @@ nodes
129129
| Jexl3Injection.java:29:39:29:53 | jexlExpr : String | semmle.label | jexlExpr : String |
130130
| Jexl3Injection.java:33:9:33:14 | script | semmle.label | script |
131131
| Jexl3Injection.java:36:50:36:64 | jexlExpr : String | semmle.label | jexlExpr : String |
132-
| Jexl3Injection.java:42:13:42:31 | callable(...) | semmle.label | callable(...) |
132+
| Jexl3Injection.java:42:13:42:18 | script | semmle.label | script |
133133
| Jexl3Injection.java:48:57:48:71 | jexlExpr : String | semmle.label | jexlExpr : String |
134134
| Jexl3Injection.java:50:40:50:47 | jexlExpr | semmle.label | jexlExpr |
135135
| Jexl3Injection.java:53:57:53:71 | jexlExpr : String | semmle.label | jexlExpr : String |
@@ -141,7 +141,7 @@ nodes
141141
| Jexl3Injection.java:70:72:70:86 | jexlExpr : String | semmle.label | jexlExpr : String |
142142
| Jexl3Injection.java:73:9:73:37 | createTemplate(...) | semmle.label | createTemplate(...) |
143143
| Jexl3Injection.java:76:54:76:68 | jexlExpr : String | semmle.label | jexlExpr : String |
144-
| Jexl3Injection.java:82:13:82:26 | callable(...) | semmle.label | callable(...) |
144+
| Jexl3Injection.java:82:13:82:13 | e | semmle.label | e |
145145
| Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
146146
| Jexl3Injection.java:94:31:94:38 | jexlExpr : String | semmle.label | jexlExpr : String |
147147
| Jexl3Injection.java:102:24:102:56 | jexlExpr : String | semmle.label | jexlExpr : String |
@@ -174,7 +174,7 @@ nodes
174174
| 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 |
175175
| 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 |
176176
| 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:31 | callable(...) | Jexl2Injection.java:76:25:76:47 | getInputStream(...) : InputStream | Jexl2Injection.java:38:13:38:31 | callable(...) | 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 |
178178
| 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 |
179179
| 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 |
180180
| 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 |
@@ -186,10 +186,10 @@ nodes
186186
| 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 |
187187
| 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 |
188188
| 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:31 | callable(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:42:13:42:31 | callable(...) | 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 |
190190
| 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 |
191191
| 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 |
192192
| 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 |
193193
| 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 |
194194
| 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:26 | callable(...) | Jexl3Injection.java:92:25:92:47 | getInputStream(...) : InputStream | Jexl3Injection.java:82:13:82:26 | callable(...) | 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)