Skip to content

Commit e4e51b5

Browse files
authored
Merge pull request github#3291 from artem-smotrakov/spel-injection
Java: Add a query for SpEL injections
2 parents ff6936c + df3adee commit e4e51b5

17 files changed

+516
-0
lines changed
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
public Object evaluate(Socket socket) throws IOException {
2+
try (BufferedReader reader = new BufferedReader(
3+
new InputStreamReader(socket.getInputStream()))) {
4+
5+
String string = reader.readLine();
6+
ExpressionParser parser = new SpelExpressionParser();
7+
Expression expression = parser.parseExpression(string);
8+
SimpleEvaluationContext context
9+
= SimpleEvaluationContext.forReadWriteDataBinding().build();
10+
return expression.getValue(context);
11+
}
12+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
The Spring Expression Language (SpEL) is a powerful expression language
7+
provided by Spring Framework. The language offers many features
8+
including invocation of methods available in the JVM.
9+
If a SpEL expression is built using attacker-controlled data,
10+
and then evaluated in a powerful context,
11+
then it may allow the attacker to run arbitrary code.
12+
</p>
13+
<p>
14+
The <code>SpelExpressionParser</code> class parses a SpEL expression string
15+
and returns an <code>Expression</code> instance
16+
that can be then evaluated by calling one of its methods.
17+
By default, an expression is evaluated in a powerful <code>StandardEvaluationContext</code>
18+
that allows the expression to access other methods available in the JVM.
19+
</p>
20+
</overview>
21+
22+
<recommendation>
23+
<p>
24+
In general, including user input in a SpEL expression should be avoided.
25+
If user input must be included in the expression,
26+
it should be then evaluated in a limited context
27+
that doesn't allow arbitrary method invocation.
28+
</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>
33+
The following example uses untrusted data to build a SpEL expression
34+
and then runs it in the default powerfull context.
35+
</p>
36+
<sample src="UnsafeSpelExpressionEvaluation.java" />
37+
38+
<p>
39+
The next example shows how an untrusted SpEL expression can be run
40+
in <code>SimpleEvaluationContext</code> that doesn't allow accessing arbitrary methods.
41+
However, it's recommended to avoid using untrusted input in SpEL expressions.
42+
</p>
43+
<sample src="SaferSpelExpressionEvaluation.java" />
44+
</example>
45+
46+
<references>
47+
<li>
48+
Spring Framework Reference Documentation:
49+
<a href="https://docs.spring.io/spring/docs/4.2.x/spring-framework-reference/html/expressions.html">Spring Expression Language (SpEL)</a>.
50+
</li>
51+
<li>
52+
OWASP:
53+
<a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
54+
</li>
55+
</references>
56+
</qhelp>
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Expression language injection (Spring)
3+
* @description Evaluation of a user-controlled Spring Expression Language (SpEL) expression
4+
* may lead to remote code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/spel-expression-injection
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import SpelInjectionLib
15+
import DataFlow::PathGraph
16+
17+
from DataFlow::PathNode source, DataFlow::PathNode sink, ExpressionInjectionConfig conf
18+
where conf.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "SpEL injection from $@.", source.getNode(), "this user input"
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.dataflow.TaintTracking2
4+
import SpringFrameworkLib
5+
6+
/**
7+
* A taint-tracking configuration for unsafe user input
8+
* that is used to construct and evaluate a SpEL expression.
9+
*/
10+
class ExpressionInjectionConfig extends TaintTracking::Configuration {
11+
ExpressionInjectionConfig() { this = "ExpressionInjectionConfig" }
12+
13+
override predicate isSource(DataFlow::Node source) {
14+
source instanceof RemoteFlowSource or
15+
source instanceof WebRequestSource
16+
}
17+
18+
override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink }
19+
20+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
21+
expressionParsingStep(node1, node2) or
22+
springPropertiesStep(node1, node2)
23+
}
24+
}
25+
26+
/**
27+
* A sink for SpEL injection vulnerabilities,
28+
* i.e. methods that run evaluation of a SpEL expression in a powerfull context.
29+
*/
30+
class ExpressionEvaluationSink extends DataFlow::ExprNode {
31+
ExpressionEvaluationSink() {
32+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
33+
m instanceof ExpressionEvaluationMethod and
34+
getExpr() = ma.getQualifier() and
35+
not exists(SafeEvaluationContextFlowConfig config |
36+
config.hasFlowTo(DataFlow::exprNode(ma.getArgument(0)))
37+
)
38+
)
39+
}
40+
}
41+
42+
/**
43+
* Holds if `node1` to `node2` is a dataflow step that parses a SpEL expression,
44+
* i.e. `parser.parseExpression(tainted)`.
45+
*/
46+
predicate expressionParsingStep(DataFlow::Node node1, DataFlow::Node node2) {
47+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
48+
m.getDeclaringType().getAnAncestor*() instanceof ExpressionParser and
49+
m.hasName("parseExpression") and
50+
ma.getAnArgument() = node1.asExpr() and
51+
node2.asExpr() = ma
52+
)
53+
}
54+
55+
/**
56+
* A configuration for safe evaluation context that may be used in expression evaluation.
57+
*/
58+
class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration {
59+
SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" }
60+
61+
override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
62+
63+
override predicate isSink(DataFlow::Node sink) {
64+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
65+
m instanceof ExpressionEvaluationMethod and
66+
ma.getArgument(0) = sink.asExpr()
67+
)
68+
}
69+
70+
override int fieldFlowBranchLimit() { result = 0 }
71+
}
72+
73+
class SafeContextSource extends DataFlow::ExprNode {
74+
SafeContextSource() {
75+
isSimpleEvaluationContextConstructorCall(getExpr()) or
76+
isSimpleEvaluationContextBuilderCall(getExpr())
77+
}
78+
}
79+
80+
/**
81+
* Holds if `expr` constructs `SimpleEvaluationContext`.
82+
*/
83+
predicate isSimpleEvaluationContextConstructorCall(Expr expr) {
84+
exists(ConstructorCall cc |
85+
cc.getConstructedType() instanceof SimpleEvaluationContext and
86+
cc = expr
87+
)
88+
}
89+
90+
/**
91+
* Holds if `expr` builds `SimpleEvaluationContext` via `SimpleEvaluationContext.Builder`,
92+
* e.g. `SimpleEvaluationContext.forReadWriteDataBinding().build()`.
93+
*/
94+
predicate isSimpleEvaluationContextBuilderCall(Expr expr) {
95+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
96+
m.getDeclaringType() instanceof SimpleEvaluationContextBuilder and
97+
m.hasName("build") and
98+
ma = expr
99+
)
100+
}
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import java
2+
import semmle.code.java.dataflow.DataFlow
3+
4+
/**
5+
* Methods that trigger evaluation of an expression.
6+
*/
7+
class ExpressionEvaluationMethod extends Method {
8+
ExpressionEvaluationMethod() {
9+
getDeclaringType() instanceof Expression and
10+
(
11+
hasName("getValue") or
12+
hasName("getValueTypeDescriptor") or
13+
hasName("getValueType") or
14+
hasName("setValue")
15+
)
16+
}
17+
}
18+
19+
/**
20+
* `WebRequest` interface is a source of tainted data.
21+
*/
22+
class WebRequestSource extends DataFlow::Node {
23+
WebRequestSource() {
24+
exists(MethodAccess ma, Method m | ma.getMethod() = m |
25+
m.getDeclaringType() instanceof WebRequest and
26+
(
27+
m.hasName("getHeader") or
28+
m.hasName("getHeaderValues") or
29+
m.hasName("getHeaderNames") or
30+
m.hasName("getParameter") or
31+
m.hasName("getParameterValues") or
32+
m.hasName("getParameterNames") or
33+
m.hasName("getParameterMap")
34+
) and
35+
ma = asExpr()
36+
)
37+
}
38+
}
39+
40+
/**
41+
* Holds if `node1` to `node2` is a dataflow step that converts `PropertyValues`
42+
* to an array of `PropertyValue`, i.e. `tainted.getPropertyValues()`.
43+
*/
44+
predicate getPropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) {
45+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
46+
node1.asExpr() = ma.getQualifier() and
47+
node2.asExpr() = ma and
48+
m.getDeclaringType() instanceof PropertyValues and
49+
m.hasName("getPropertyValues")
50+
)
51+
}
52+
53+
/**
54+
* Holds if `node1` to `node2` is a dataflow step that constructs `MutablePropertyValues`,
55+
* i.e. `new MutablePropertyValues(tainted)`.
56+
*/
57+
predicate createMutablePropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) {
58+
exists(ConstructorCall cc | cc.getConstructedType() instanceof MutablePropertyValues |
59+
node1.asExpr() = cc.getAnArgument() and
60+
node2.asExpr() = cc
61+
)
62+
}
63+
64+
/**
65+
* Holds if `node1` to `node2` is a dataflow step that returns a name of `PropertyValue`,
66+
* i.e. `tainted.getName()`.
67+
*/
68+
predicate getPropertyNameStep(DataFlow::Node node1, DataFlow::Node node2) {
69+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
70+
node1.asExpr() = ma.getQualifier() and
71+
node2.asExpr() = ma and
72+
m.getDeclaringType() instanceof PropertyValue and
73+
m.hasName("getName")
74+
)
75+
}
76+
77+
/**
78+
* Holds if `node1` to `node2` is a dataflow step that converts `MutablePropertyValues`
79+
* to a list of `PropertyValue`, i.e. `tainted.getPropertyValueList()`.
80+
*/
81+
predicate getPropertyValueListStep(DataFlow::Node node1, DataFlow::Node node2) {
82+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
83+
node1.asExpr() = ma.getQualifier() and
84+
node2.asExpr() = ma and
85+
m.getDeclaringType() instanceof MutablePropertyValues and
86+
m.hasName("getPropertyValueList")
87+
)
88+
}
89+
90+
/**
91+
* Holds if `node1` to `node2` is one of the dataflow steps that propagate
92+
* tainted data via Spring properties.
93+
*/
94+
predicate springPropertiesStep(DataFlow::Node node1, DataFlow::Node node2) {
95+
createMutablePropertyValuesStep(node1, node2) or
96+
getPropertyNameStep(node1, node2) or
97+
getPropertyValuesStep(node1, node2) or
98+
getPropertyValueListStep(node1, node2)
99+
}
100+
101+
class PropertyValue extends RefType {
102+
PropertyValue() { hasQualifiedName("org.springframework.beans", "PropertyValue") }
103+
}
104+
105+
class PropertyValues extends RefType {
106+
PropertyValues() { hasQualifiedName("org.springframework.beans", "PropertyValues") }
107+
}
108+
109+
class MutablePropertyValues extends RefType {
110+
MutablePropertyValues() { hasQualifiedName("org.springframework.beans", "MutablePropertyValues") }
111+
}
112+
113+
class SimpleEvaluationContext extends RefType {
114+
SimpleEvaluationContext() {
115+
hasQualifiedName("org.springframework.expression.spel.support", "SimpleEvaluationContext")
116+
}
117+
}
118+
119+
class SimpleEvaluationContextBuilder extends RefType {
120+
SimpleEvaluationContextBuilder() {
121+
hasQualifiedName("org.springframework.expression.spel.support",
122+
"SimpleEvaluationContext$Builder")
123+
}
124+
}
125+
126+
class WebRequest extends RefType {
127+
WebRequest() { hasQualifiedName("org.springframework.web.context.request", "WebRequest") }
128+
}
129+
130+
class Expression extends RefType {
131+
Expression() { hasQualifiedName("org.springframework.expression", "Expression") }
132+
}
133+
134+
class ExpressionParser extends RefType {
135+
ExpressionParser() { hasQualifiedName("org.springframework.expression", "ExpressionParser") }
136+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
public Object evaluate(Socket socket) throws IOException {
2+
try (BufferedReader reader = new BufferedReader(
3+
new InputStreamReader(socket.getInputStream()))) {
4+
5+
String string = reader.readLine();
6+
ExpressionParser parser = new SpelExpressionParser();
7+
Expression expression = parser.parseExpression(string);
8+
return expression.getValue();
9+
}
10+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
edges
2+
| SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | SpelInjection.java:23:5:23:14 | expression |
3+
| SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | SpelInjection.java:34:5:34:14 | expression |
4+
| SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | SpelInjection.java:48:5:48:14 | expression |
5+
| SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | SpelInjection.java:59:5:59:14 | expression |
6+
| SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | SpelInjection.java:70:5:70:14 | expression |
7+
| SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | SpelInjection.java:83:5:83:14 | expression |
8+
nodes
9+
| SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
10+
| SpelInjection.java:23:5:23:14 | expression | semmle.label | expression |
11+
| SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
12+
| SpelInjection.java:34:5:34:14 | expression | semmle.label | expression |
13+
| SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
14+
| SpelInjection.java:48:5:48:14 | expression | semmle.label | expression |
15+
| SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
16+
| SpelInjection.java:59:5:59:14 | expression | semmle.label | expression |
17+
| SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
18+
| SpelInjection.java:70:5:70:14 | expression | semmle.label | expression |
19+
| SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
20+
| SpelInjection.java:83:5:83:14 | expression | semmle.label | expression |
21+
#select
22+
| SpelInjection.java:23:5:23:14 | expression | SpelInjection.java:15:22:15:44 | getInputStream(...) : InputStream | SpelInjection.java:23:5:23:14 | expression | SpEL injection from $@. | SpelInjection.java:15:22:15:44 | getInputStream(...) | this user input |
23+
| SpelInjection.java:34:5:34:14 | expression | SpelInjection.java:27:22:27:44 | getInputStream(...) : InputStream | SpelInjection.java:34:5:34:14 | expression | SpEL injection from $@. | SpelInjection.java:27:22:27:44 | getInputStream(...) | this user input |
24+
| SpelInjection.java:48:5:48:14 | expression | SpelInjection.java:38:22:38:44 | getInputStream(...) : InputStream | SpelInjection.java:48:5:48:14 | expression | SpEL injection from $@. | SpelInjection.java:38:22:38:44 | getInputStream(...) | this user input |
25+
| SpelInjection.java:59:5:59:14 | expression | SpelInjection.java:52:22:52:44 | getInputStream(...) : InputStream | SpelInjection.java:59:5:59:14 | expression | SpEL injection from $@. | SpelInjection.java:52:22:52:44 | getInputStream(...) | this user input |
26+
| SpelInjection.java:70:5:70:14 | expression | SpelInjection.java:63:22:63:44 | getInputStream(...) : InputStream | SpelInjection.java:70:5:70:14 | expression | SpEL injection from $@. | SpelInjection.java:63:22:63:44 | getInputStream(...) | this user input |
27+
| SpelInjection.java:83:5:83:14 | expression | SpelInjection.java:74:22:74:44 | getInputStream(...) : InputStream | SpelInjection.java:83:5:83:14 | expression | SpEL injection from $@. | SpelInjection.java:74:22:74:44 | getInputStream(...) | this user input |

0 commit comments

Comments
 (0)