Skip to content

Commit

Permalink
GH-835 Fix RoutingFunction SpEL evaluation
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@olegz
olegz committed Mar 24, 2022
1 parent b02fe24 commit 0e89ee2
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 15 deletions.
14 changes: 11 additions & 3 deletions ...text/src/main/java/org/springframework/cloud/function/context/config/RoutingFunction.java
View file
Expand Up @@ -34,12 +34,13 @@
import org.springframework.expression.BeanResolver;
import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.DataBindingPropertyAccessor;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.messaging.Message;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;


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

private final StandardEvaluationContext evalContext = new StandardEvaluationContext();

private final SimpleEvaluationContext headerEvalContext = SimpleEvaluationContext
.forPropertyAccessors(DataBindingPropertyAccessor.forReadOnlyAccess()).build();

private final SpelExpressionParser spelParser = new SpelExpressionParser();

private final FunctionCatalog functionCatalog;
Expand Down Expand Up @@ -124,7 +128,7 @@ private Object route(Object input, boolean originalInputIsPublisher) {
}
}
else if (StringUtils.hasText((String) message.getHeaders().get("spring.cloud.function.routing-expression"))) {
function = this.functionFromExpression((String) message.getHeaders().get("spring.cloud.function.routing-expression"), message);
function = this.functionFromExpression((String) message.getHeaders().get("spring.cloud.function.routing-expression"), message, true);
if (function.isInputTypePublisher()) {
this.assertOriginalInputIsNotPublisher(originalInputIsPublisher);
}
Expand Down Expand Up @@ -193,12 +197,16 @@ private FunctionInvocationWrapper functionFromDefinition(String definition) {
}

private FunctionInvocationWrapper functionFromExpression(String routingExpression, Object input) {
return functionFromExpression(routingExpression, input, false);
}

private FunctionInvocationWrapper functionFromExpression(String routingExpression, Object input, boolean isViaHeader) {
Expression expression = spelParser.parseExpression(routingExpression);
if (input instanceof Message) {
input = MessageUtils.toCaseInsensitiveHeadersStructure((Message<?>) input);
}

String functionName = expression.getValue(this.evalContext, input, String.class);
String functionName = isViaHeader ? expression.getValue(this.headerEvalContext, input, String.class) : expression.getValue(this.evalContext, input, String.class);
Assert.hasText(functionName, "Failed to resolve function name based on routing expression '" + functionProperties.getRoutingExpression() + "'");
FunctionInvocationWrapper function = functionCatalog.lookup(functionName);
Assert.notNull(function, "Failed to lookup function to route to based on the expression '"
Expand Down
41 changes: 29 additions & 12 deletions ...src/test/java/org/springframework/cloud/function/context/config/RoutingFunctionTests.java
View file
Expand Up @@ -35,6 +35,7 @@
import org.springframework.messaging.support.MessageBuilder;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;

/**
*
Expand Down Expand Up @@ -91,10 +92,7 @@ public void testRoutingReactiveInputWithReactiveFunctionAndDefinitionMessageHead
.setHeader(FunctionProperties.PREFIX + ".definition", "echoFlux").build();
Flux resultFlux = (Flux) function.apply(Flux.just(message));

StepVerifier
.create(resultFlux)
.expectError()
.verify();
StepVerifier.create(resultFlux).expectError().verify();
}

@SuppressWarnings({ "unchecked", "rawtypes" })
Expand All @@ -106,10 +104,27 @@ public void testRoutingReactiveInputWithReactiveFunctionAndExpressionMessageHead
Message<String> message = MessageBuilder.withPayload("hello")
.setHeader(FunctionProperties.PREFIX + ".routing-expression", "'echoFlux'").build();
Flux resultFlux = (Flux) function.apply(Flux.just(message));
StepVerifier
.create(resultFlux)
.expectError()
.verify();
StepVerifier.create(resultFlux).expectError().verify();
}

@SuppressWarnings({ "unchecked", "rawtypes" })
@Test
public void failWithHeaderProvidedExpressionAccessingRuntime() {
FunctionCatalog functionCatalog = this.configureCatalog();
Function function = functionCatalog.lookup(RoutingFunction.FUNCTION_NAME);
assertThat(function).isNotNull();
Message<String> message = MessageBuilder.withPayload("hello")
.setHeader(FunctionProperties.PREFIX + ".routing-expression",
"T(java.lang.Runtime).getRuntime().exec(\"open -a calculator.app\")")
.build();
try {
function.apply(message);
fail();
}
catch (Exception e) {
assertThat(e.getMessage()).isEqualTo("EL1005E: Type cannot be found 'java.lang.Runtime'");
}

}

@SuppressWarnings({ "unchecked", "rawtypes" })
Expand Down Expand Up @@ -151,7 +166,8 @@ public void testInvocationWithMessageAndRoutingExpressionCaseInsensitive() {
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testInvocationWithRoutingBeanExpression() {
System.setProperty(FunctionProperties.PREFIX + ".routing-expression", "@reverse.apply(#root.getHeaders().get('func'))");
System.setProperty(FunctionProperties.PREFIX + ".routing-expression",
"@reverse.apply(#root.getHeaders().get('func'))");
FunctionCatalog functionCatalog = this.configureCatalog();
Function function = functionCatalog.lookup(RoutingFunction.FUNCTION_NAME);
assertThat(function).isNotNull();
Expand All @@ -170,16 +186,17 @@ public void testOtherExpectedFailures() {
Assertions.fail();
}
catch (Exception e) {
//ignore
// ignore
}

// non existing function
try {
function.apply(MessageBuilder.withPayload("hello").setHeader(FunctionProperties.PREFIX + ".definition", "blah").build());
function.apply(MessageBuilder.withPayload("hello")
.setHeader(FunctionProperties.PREFIX + ".definition", "blah").build());
Assertions.fail();
}
catch (Exception e) {
//ignore
// ignore
}
}

Expand Down

0 comments on commit 0e89ee2

Please sign in to comment.