Skip to content

Commit 0e89ee2

Browse files
committed
GH-835 Fix RoutingFunction SpEL evaluation
This fix ensures that any SpEL expression provided via Message Headers is not evaluated in the scope of StandardEvaluationContext as it can result in execution of arbitrary code via java Runtime Resolves #835
1 parent b02fe24 commit 0e89ee2

File tree

2 files changed

+40
-15
lines changed

2 files changed

+40
-15
lines changed

spring-cloud-function-context/src/main/java/org/springframework/cloud/function/context/config/RoutingFunction.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,13 @@
3434
import org.springframework.expression.BeanResolver;
3535
import org.springframework.expression.Expression;
3636
import org.springframework.expression.spel.standard.SpelExpressionParser;
37+
import org.springframework.expression.spel.support.DataBindingPropertyAccessor;
38+
import org.springframework.expression.spel.support.SimpleEvaluationContext;
3739
import org.springframework.expression.spel.support.StandardEvaluationContext;
3840
import org.springframework.messaging.Message;
3941
import org.springframework.util.Assert;
4042
import org.springframework.util.StringUtils;
4143

42-
4344
/**
4445
* An implementation of Function which acts as a gateway/router by actually
4546
* delegating incoming invocation to a function specified .. .
@@ -60,6 +61,9 @@ public class RoutingFunction implements Function<Object, Object> {
6061

6162
private final StandardEvaluationContext evalContext = new StandardEvaluationContext();
6263

64+
private final SimpleEvaluationContext headerEvalContext = SimpleEvaluationContext
65+
.forPropertyAccessors(DataBindingPropertyAccessor.forReadOnlyAccess()).build();
66+
6367
private final SpelExpressionParser spelParser = new SpelExpressionParser();
6468

6569
private final FunctionCatalog functionCatalog;
@@ -124,7 +128,7 @@ private Object route(Object input, boolean originalInputIsPublisher) {
124128
}
125129
}
126130
else if (StringUtils.hasText((String) message.getHeaders().get("spring.cloud.function.routing-expression"))) {
127-
function = this.functionFromExpression((String) message.getHeaders().get("spring.cloud.function.routing-expression"), message);
131+
function = this.functionFromExpression((String) message.getHeaders().get("spring.cloud.function.routing-expression"), message, true);
128132
if (function.isInputTypePublisher()) {
129133
this.assertOriginalInputIsNotPublisher(originalInputIsPublisher);
130134
}
@@ -193,12 +197,16 @@ private FunctionInvocationWrapper functionFromDefinition(String definition) {
193197
}
194198

195199
private FunctionInvocationWrapper functionFromExpression(String routingExpression, Object input) {
200+
return functionFromExpression(routingExpression, input, false);
201+
}
202+
203+
private FunctionInvocationWrapper functionFromExpression(String routingExpression, Object input, boolean isViaHeader) {
196204
Expression expression = spelParser.parseExpression(routingExpression);
197205
if (input instanceof Message) {
198206
input = MessageUtils.toCaseInsensitiveHeadersStructure((Message<?>) input);
199207
}
200208

201-
String functionName = expression.getValue(this.evalContext, input, String.class);
209+
String functionName = isViaHeader ? expression.getValue(this.headerEvalContext, input, String.class) : expression.getValue(this.evalContext, input, String.class);
202210
Assert.hasText(functionName, "Failed to resolve function name based on routing expression '" + functionProperties.getRoutingExpression() + "'");
203211
FunctionInvocationWrapper function = functionCatalog.lookup(functionName);
204212
Assert.notNull(function, "Failed to lookup function to route to based on the expression '"

spring-cloud-function-context/src/test/java/org/springframework/cloud/function/context/config/RoutingFunctionTests.java

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.springframework.messaging.support.MessageBuilder;
3636

3737
import static org.assertj.core.api.Assertions.assertThat;
38+
import static org.junit.Assert.fail;
3839

3940
/**
4041
*
@@ -91,10 +92,7 @@ public void testRoutingReactiveInputWithReactiveFunctionAndDefinitionMessageHead
9192
.setHeader(FunctionProperties.PREFIX + ".definition", "echoFlux").build();
9293
Flux resultFlux = (Flux) function.apply(Flux.just(message));
9394

94-
StepVerifier
95-
.create(resultFlux)
96-
.expectError()
97-
.verify();
95+
StepVerifier.create(resultFlux).expectError().verify();
9896
}
9997

10098
@SuppressWarnings({ "unchecked", "rawtypes" })
@@ -106,10 +104,27 @@ public void testRoutingReactiveInputWithReactiveFunctionAndExpressionMessageHead
106104
Message<String> message = MessageBuilder.withPayload("hello")
107105
.setHeader(FunctionProperties.PREFIX + ".routing-expression", "'echoFlux'").build();
108106
Flux resultFlux = (Flux) function.apply(Flux.just(message));
109-
StepVerifier
110-
.create(resultFlux)
111-
.expectError()
112-
.verify();
107+
StepVerifier.create(resultFlux).expectError().verify();
108+
}
109+
110+
@SuppressWarnings({ "unchecked", "rawtypes" })
111+
@Test
112+
public void failWithHeaderProvidedExpressionAccessingRuntime() {
113+
FunctionCatalog functionCatalog = this.configureCatalog();
114+
Function function = functionCatalog.lookup(RoutingFunction.FUNCTION_NAME);
115+
assertThat(function).isNotNull();
116+
Message<String> message = MessageBuilder.withPayload("hello")
117+
.setHeader(FunctionProperties.PREFIX + ".routing-expression",
118+
"T(java.lang.Runtime).getRuntime().exec(\"open -a calculator.app\")")
119+
.build();
120+
try {
121+
function.apply(message);
122+
fail();
123+
}
124+
catch (Exception e) {
125+
assertThat(e.getMessage()).isEqualTo("EL1005E: Type cannot be found 'java.lang.Runtime'");
126+
}
127+
113128
}
114129

115130
@SuppressWarnings({ "unchecked", "rawtypes" })
@@ -151,7 +166,8 @@ public void testInvocationWithMessageAndRoutingExpressionCaseInsensitive() {
151166
@SuppressWarnings({ "rawtypes", "unchecked" })
152167
@Test
153168
public void testInvocationWithRoutingBeanExpression() {
154-
System.setProperty(FunctionProperties.PREFIX + ".routing-expression", "@reverse.apply(#root.getHeaders().get('func'))");
169+
System.setProperty(FunctionProperties.PREFIX + ".routing-expression",
170+
"@reverse.apply(#root.getHeaders().get('func'))");
155171
FunctionCatalog functionCatalog = this.configureCatalog();
156172
Function function = functionCatalog.lookup(RoutingFunction.FUNCTION_NAME);
157173
assertThat(function).isNotNull();
@@ -170,16 +186,17 @@ public void testOtherExpectedFailures() {
170186
Assertions.fail();
171187
}
172188
catch (Exception e) {
173-
//ignore
189+
// ignore
174190
}
175191

176192
// non existing function
177193
try {
178-
function.apply(MessageBuilder.withPayload("hello").setHeader(FunctionProperties.PREFIX + ".definition", "blah").build());
194+
function.apply(MessageBuilder.withPayload("hello")
195+
.setHeader(FunctionProperties.PREFIX + ".definition", "blah").build());
179196
Assertions.fail();
180197
}
181198
catch (Exception e) {
182-
//ignore
199+
// ignore
183200
}
184201
}
185202

0 commit comments

Comments
 (0)