Skip to content

Commit fa36ba9

Browse files
smowtonsmowton
authored
Merge pull request github#5471 from artem-smotrakov/el-injection
Java: Query for detecting Jakarta Expression Language injections
2 parents 972cc47 + 97186b3 commit fa36ba9

21 files changed

+438
-15
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
4+
/**
5+
* Holds if `fromNode` to `toNode` is a dataflow step that returns data from
6+
* a bean by calling one of its getters.
7+
*/
8+
predicate hasGetterFlow(DataFlow::Node fromNode, DataFlow::Node toNode) {
9+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
10+
m instanceof GetterMethod and
11+
ma.getQualifier() = fromNode.asExpr() and
12+
ma = toNode.asExpr()
13+
)
14+
}

‎java/ql/src/experimental/Security/CWE/CWE-094/JakartaExpressionInjection.qhelp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Jakarta Expression Language (EL) is an expression language for Java applications.
7+
There is a single language specification and multiple implementations
8+
such as Glassfish, Juel, Apache Commons EL, etc.
9+
The language allows invocation of methods available in the JVM.
10+
If an expression is built using attacker-controlled data,
11+
and then evaluated, it may allow the attacker to run arbitrary code.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
It is generally recommended to avoid using untrusted data in an EL expression.
18+
Before using untrusted data to build an EL expression, the data should be validated
19+
to ensure it is not evaluated as expression language. If the EL implementation offers
20+
configuring a sandbox for EL expressions, they should be run in a restrictive sandbox
21+
that allows accessing only explicitly allowed classes. If the EL implementation
22+
does not support sandboxing, consider using other expression language implementations
23+
with sandboxing capabilities such as Apache Commons JEXL or the Spring Expression Language.
24+
</p>
25+
</recommendation>
26+
27+
<example>
28+
<p>
29+
The following example shows how untrusted data is used to build and run an expression
30+
using the JUEL interpreter:
31+
</p>
32+
<sample src="UnsafeExpressionEvaluationWithJuel.java" />
33+
34+
<p>
35+
JUEL does not support running expressions in a sandbox. To prevent running arbitrary code,
36+
incoming data has to be checked before including it in an expression. The next example
37+
uses a Regex pattern to check whether a user tries to run an allowed expression or not:
38+
</p>
39+
<sample src="SaferExpressionEvaluationWithJuel.java" />
40+
41+
</example>
42+
43+
<references>
44+
<li>
45+
Eclipse Foundation:
46+
<a href="https://projects.eclipse.org/projects/ee4j.el">Jakarta Expression Language</a>.
47+
</li>
48+
<li>
49+
Jakarta EE documentation:
50+
<a href="https://javadoc.io/doc/jakarta.el/jakarta.el-api/latest/index.html">Jakarta Expression Language API</a>
51+
</li>
52+
<li>
53+
OWASP:
54+
<a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
55+
</li>
56+
<li>
57+
JUEL:
58+
<a href="http://juel.sourceforge.net">Home page</a>
59+
</li>
60+
</references>
61+
</qhelp>

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Jakarta Expression Language injection
3+
* @description Evaluation of a user-controlled expression
4+
* may lead to arbitrary code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/javaee-expression-injection
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import JakartaExpressionInjectionLib
15+
import DataFlow::PathGraph
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, JakartaExpressionInjectionConfig conf
18+
where conf.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "Jakarta Expression Language injection from $@.",
20+
source.getNode(), "this user input"

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import java
2+
import FlowUtils
3+
import semmle.code.java.dataflow.FlowSources
4+
import semmle.code.java.dataflow.TaintTracking
5+
6+
/**
7+
* A taint-tracking configuration for unsafe user input
8+
* that is used to construct and evaluate an expression.
9+
*/
10+
class JakartaExpressionInjectionConfig extends TaintTracking::Configuration {
11+
JakartaExpressionInjectionConfig() { this = "JakartaExpressionInjectionConfig" }
12+
13+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
14+
15+
override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink }
16+
17+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
18+
any(TaintPropagatingCall c).taintFlow(fromNode, toNode) or
19+
hasGetterFlow(fromNode, toNode)
20+
}
21+
}
22+
23+
/**
24+
* A sink for Expresssion Language injection vulnerabilities,
25+
* i.e. method calls that run evaluation of an expression.
26+
*/
27+
private class ExpressionEvaluationSink extends DataFlow::ExprNode {
28+
ExpressionEvaluationSink() {
29+
exists(MethodAccess ma, Method m, Expr taintFrom |
30+
ma.getMethod() = m and taintFrom = this.asExpr()
31+
|
32+
m.getDeclaringType() instanceof ValueExpression and
33+
m.hasName(["getValue", "setValue"]) and
34+
ma.getQualifier() = taintFrom
35+
or
36+
m.getDeclaringType() instanceof MethodExpression and
37+
m.hasName("invoke") and
38+
ma.getQualifier() = taintFrom
39+
or
40+
m.getDeclaringType() instanceof LambdaExpression and
41+
m.hasName("invoke") and
42+
ma.getQualifier() = taintFrom
43+
or
44+
m.getDeclaringType() instanceof ELProcessor and
45+
m.hasName(["eval", "getValue", "setValue"]) and
46+
ma.getArgument(0) = taintFrom
47+
or
48+
m.getDeclaringType() instanceof ELProcessor and
49+
m.hasName("setVariable") and
50+
ma.getArgument(1) = taintFrom
51+
)
52+
}
53+
}
54+
55+
/**
56+
* Defines method calls that propagate tainted expressions.
57+
*/
58+
private class TaintPropagatingCall extends Call {
59+
Expr taintFromExpr;
60+
61+
TaintPropagatingCall() {
62+
taintFromExpr = this.getArgument(1) and
63+
(
64+
exists(Method m | this.(MethodAccess).getMethod() = m |
65+
m.getDeclaringType() instanceof ExpressionFactory and
66+
m.hasName(["createValueExpression", "createMethodExpression"]) and
67+
taintFromExpr.getType() instanceof TypeString
68+
)
69+
or
70+
exists(Constructor c | this.(ConstructorCall).getConstructor() = c |
71+
c.getDeclaringType() instanceof LambdaExpression and
72+
taintFromExpr.getType() instanceof ValueExpression
73+
)
74+
)
75+
}
76+
77+
/**
78+
* Holds if `fromNode` to `toNode` is a dataflow step that propagates
79+
* tainted data.
80+
*/
81+
predicate taintFlow(DataFlow::Node fromNode, DataFlow::Node toNode) {
82+
fromNode.asExpr() = taintFromExpr and toNode.asExpr() = this
83+
}
84+
}
85+
86+
private class JakartaType extends RefType {
87+
JakartaType() { getPackage().hasName(["javax.el", "jakarta.el"]) }
88+
}
89+
90+
private class ELProcessor extends JakartaType {
91+
ELProcessor() { hasName("ELProcessor") }
92+
}
93+
94+
private class ExpressionFactory extends JakartaType {
95+
ExpressionFactory() { hasName("ExpressionFactory") }
96+
}
97+
98+
private class ValueExpression extends JakartaType {
99+
ValueExpression() { hasName("ValueExpression") }
100+
}
101+
102+
private class MethodExpression extends JakartaType {
103+
MethodExpression() { hasName("MethodExpression") }
104+
}
105+
106+
private class LambdaExpression extends JakartaType {
107+
LambdaExpression() { hasName("LambdaExpression") }
108+
}

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import java
2+
import FlowUtils
23
import semmle.code.java.dataflow.FlowSources
34
import semmle.code.java.dataflow.TaintTracking
45

@@ -16,7 +17,7 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
1617

1718
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
1819
any(TaintPropagatingJexlMethodCall c).taintFlow(fromNode, toNode) or
19-
returnsDataFromBean(fromNode, toNode)
20+
hasGetterFlow(fromNode, toNode)
2021
}
2122
}
2223

@@ -152,18 +153,6 @@ private predicate createsJexlEngine(DataFlow::Node fromNode, DataFlow::Node toNo
152153
)
153154
}
154155

155-
/**
156-
* Holds if `fromNode` to `toNode` is a dataflow step that returns data from
157-
* a bean by calling one of its getters.
158-
*/
159-
private predicate returnsDataFromBean(DataFlow::Node fromNode, DataFlow::Node toNode) {
160-
exists(MethodAccess ma, Method m | ma.getMethod() = m |
161-
m instanceof GetterMethod and
162-
ma.getQualifier() = fromNode.asExpr() and
163-
ma = toNode.asExpr()
164-
)
165-
}
166-
167156
/**
168157
* A methods in the `JexlEngine` class that gets or sets a property with a JEXL expression.
169158
*/

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
String input = getRemoteUserInput();
2+
String pattern = "(inside|outside)\\.(temperature|humidity)";
3+
if (!input.matches(pattern)) {
4+
throw new IllegalArgumentException("Unexpected expression");
5+
}
6+
String expression = "${" + input + "}";
7+
ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl();
8+
ValueExpression e = factory.createValueExpression(context, expression, Object.class);
9+
SimpleContext context = getContext();
10+
Object result = e.getValue(context);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
String expression = "${" + getRemoteUserInput() + "}";
2+
ExpressionFactory factory = new de.odysseus.el.ExpressionFactoryImpl();
3+
ValueExpression e = factory.createValueExpression(context, expression, Object.class);
4+
SimpleContext context = getContext();
5+
Object result = e.getValue(context);

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
edges
2+
| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:25:31:25:40 | expression : String |
3+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:32:24:32:33 | expression : String |
4+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:40:24:40:33 | expression : String |
5+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:48:24:48:33 | expression : String |
6+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:59:24:59:33 | expression : String |
7+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:67:24:67:33 | expression : String |
8+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:75:24:75:33 | expression : String |
9+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:85:24:85:33 | expression : String |
10+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:95:24:95:33 | expression : String |
11+
| JakartaExpressionInjection.java:32:24:32:33 | expression : String | JakartaExpressionInjection.java:34:28:34:37 | expression |
12+
| JakartaExpressionInjection.java:40:24:40:33 | expression : String | JakartaExpressionInjection.java:42:32:42:41 | expression |
13+
| JakartaExpressionInjection.java:48:24:48:33 | expression : String | JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression |
14+
| JakartaExpressionInjection.java:59:24:59:33 | expression : String | JakartaExpressionInjection.java:61:32:61:41 | expression |
15+
| JakartaExpressionInjection.java:67:24:67:33 | expression : String | JakartaExpressionInjection.java:69:43:69:52 | expression |
16+
| JakartaExpressionInjection.java:75:24:75:33 | expression : String | JakartaExpressionInjection.java:79:13:79:13 | e |
17+
| JakartaExpressionInjection.java:85:24:85:33 | expression : String | JakartaExpressionInjection.java:89:13:89:13 | e |
18+
| JakartaExpressionInjection.java:95:24:95:33 | expression : String | JakartaExpressionInjection.java:99:13:99:13 | e |
19+
nodes
20+
| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
21+
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | semmle.label | expression : String |
22+
| JakartaExpressionInjection.java:32:24:32:33 | expression : String | semmle.label | expression : String |
23+
| JakartaExpressionInjection.java:34:28:34:37 | expression | semmle.label | expression |
24+
| JakartaExpressionInjection.java:40:24:40:33 | expression : String | semmle.label | expression : String |
25+
| JakartaExpressionInjection.java:42:32:42:41 | expression | semmle.label | expression |
26+
| JakartaExpressionInjection.java:48:24:48:33 | expression : String | semmle.label | expression : String |
27+
| JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | semmle.label | lambdaExpression |
28+
| JakartaExpressionInjection.java:59:24:59:33 | expression : String | semmle.label | expression : String |
29+
| JakartaExpressionInjection.java:61:32:61:41 | expression | semmle.label | expression |
30+
| JakartaExpressionInjection.java:67:24:67:33 | expression : String | semmle.label | expression : String |
31+
| JakartaExpressionInjection.java:69:43:69:52 | expression | semmle.label | expression |
32+
| JakartaExpressionInjection.java:75:24:75:33 | expression : String | semmle.label | expression : String |
33+
| JakartaExpressionInjection.java:79:13:79:13 | e | semmle.label | e |
34+
| JakartaExpressionInjection.java:85:24:85:33 | expression : String | semmle.label | expression : String |
35+
| JakartaExpressionInjection.java:89:13:89:13 | e | semmle.label | e |
36+
| JakartaExpressionInjection.java:95:24:95:33 | expression : String | semmle.label | expression : String |
37+
| JakartaExpressionInjection.java:99:13:99:13 | e | semmle.label | e |
38+
#select
39+
| JakartaExpressionInjection.java:34:28:34:37 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:34:28:34:37 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
40+
| JakartaExpressionInjection.java:42:32:42:41 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:42:32:42:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
41+
| JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:53:13:53:28 | lambdaExpression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
42+
| JakartaExpressionInjection.java:61:32:61:41 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:61:32:61:41 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
43+
| JakartaExpressionInjection.java:69:43:69:52 | expression | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:69:43:69:52 | expression | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
44+
| JakartaExpressionInjection.java:79:13:79:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:79:13:79:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
45+
| JakartaExpressionInjection.java:89:13:89:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:89:13:89:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |
46+
| JakartaExpressionInjection.java:99:13:99:13 | e | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:99:13:99:13 | e | Jakarta Expression Language injection from $@. | JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) | this user input |

0 commit comments

Comments
 (0)