From 7f73677bc839794e5f4e698881bee4538ac8b286 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 6 Apr 2018 17:06:47 +0200 Subject: [PATCH] Spel security fix; fixes gh-917 --- docs/src/main/asciidoc/spring-cloud-sleuth.adoc | 6 ++++-- pom.xml | 4 ++-- .../annotation/SpelTagValueExpressionResolver.java | 6 +++++- .../SpelTagValueExpressionResolverTests.java | 10 ++++++++-- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 9b1fbf74c..50a1d5064 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -294,7 +294,9 @@ Precedence is: - try with the bean of `TagValueResolver` type and provided name - if one hasn't provided the bean name, try to evaluate an expression. We're searching for a `TagValueExpressionResolver` bean. -The default implementation uses SPEL expression resolution. +The default implementation uses SPEL expression resolution. If we do not find any expression to evaluate, return the `toString()` +value of the parameter. +**IMPORTANT** You can only reference properties from the SPEL expression. Method execution is not allowed due to security constraints. - if one hasn't provided any expression to evaluate just return a `toString()` value of the parameter ==== Custom extractor @@ -950,4 +952,4 @@ class ReporterConfiguration { You can find the running examples deployed in the https://run.pivotal.io/[Pivotal Web Services]. Check them out in the following links: - http://docssleuth-zipkin-server.cfapps.io/[Zipkin for apps presented in the samples to the top] -- http://docsbrewing-zipkin-server.cfapps.io/[Zipkin for Brewery on PWS], its https://github.com/spring-cloud-samples/brewery[Github Code] +- http://docsbrewing-zipkin-server.cfapps.io/[Zipkin for Brewery on PWS], its https://github.com/spring-cloud-samples/brewery[Github Code] \ No newline at end of file diff --git a/pom.xml b/pom.xml index 10fef99cc..55e06e63e 100644 --- a/pom.xml +++ b/pom.xml @@ -13,7 +13,7 @@ org.springframework.cloud spring-cloud-build - 1.3.8.RELEASE + 1.3.9.BUILD-SNAPSHOT @@ -241,7 +241,7 @@ 1.8 2.19.1 2.17 - 1.3.8.RELEASE + 1.3.9.BUILD-SNAPSHOT 1.3.3.BUILD-SNAPSHOT Ditmars.BUILD-SNAPSHOT 1.4.4.BUILD-SNAPSHOT diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolver.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolver.java index 711e5bd62..99bda74b8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolver.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolver.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.SimpleEvaluationContext; /** * Uses SPEL to evaluate the expression. If an exception is thrown will return @@ -37,9 +38,12 @@ class SpelTagValueExpressionResolver implements TagValueExpressionResolver { @Override public String resolve(String expression, Object parameter) { try { + SimpleEvaluationContext context = SimpleEvaluationContext + .forReadOnlyDataBinding() + .build(); ExpressionParser expressionParser = new SpelExpressionParser(); Expression expressionToEvaluate = expressionParser.parseExpression(expression); - return expressionToEvaluate.getValue(parameter, String.class); + return expressionToEvaluate.getValue(context, parameter, String.class); } catch (Exception e) { log.error("Exception occurred while tying to evaluate the SPEL expression [" + expression + "]", e); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolverTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolverTests.java index e178846e7..16d008758 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolverTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SpelTagValueExpressionResolverTests.java @@ -27,10 +27,16 @@ public class SpelTagValueExpressionResolverTests { @Test public void should_use_spel_to_resolve_a_value() throws Exception { SpelTagValueExpressionResolver resolver = new SpelTagValueExpressionResolver(); + MyObject myObject = new MyObject(); + myObject.name = "hello"; - String resolved = resolver.resolve("length() + 1", "foo"); + String resolved = resolver.resolve("name + ' world'", myObject); - then(resolved).isEqualTo("4"); + then(resolved).isEqualTo("hello world"); + } + + public static class MyObject { + public String name; } @Test