Permalink
Browse files

Prevent recursive placeholders in whitelabel views

This change prevents placeholders in model values from being
recursively replaced (which is a feature of the placeholder
utilities in Spring Core), so that user-provided data
(e.g. an invalid response_type or a leaky exception message)
cannot be used to inject SpEL into the view.

N.B. this only affects apps that are using the whitelabel views
for approval and error pages (i.e. probably nothing in
production).
  • Loading branch information...
1 parent 4f01e4a commit fff77d3fea477b566bcacfbfc95f85821a2bdc2d @dsyer dsyer committed Mar 12, 2016
@@ -23,6 +23,7 @@
import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
+import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
import org.springframework.util.PropertyPlaceholderHelper;
import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver;
import org.springframework.web.servlet.View;
@@ -35,19 +36,19 @@
class SpelView implements View {
private final String template;
+
+ private final String prefix;
private final SpelExpressionParser parser = new SpelExpressionParser();
private final StandardEvaluationContext context = new StandardEvaluationContext();
- private PropertyPlaceholderHelper helper;
-
private PlaceholderResolver resolver;
public SpelView(String template) {
this.template = template;
+ this.prefix = new RandomValueStringGenerator().generate() + "{";
this.context.addPropertyAccessor(new MapAccessor());
- this.helper = new PropertyPlaceholderHelper("${", "}");
this.resolver = new PlaceholderResolver() {
public String resolvePlaceholder(String name) {
Expression expression = parser.parseExpression(name);
@@ -68,7 +69,10 @@ public void render(Map<String, ?> model, HttpServletRequest request, HttpServlet
.getPath();
map.put("path", (Object) path==null ? "" : path);
context.setRootObject(map);
- String result = helper.replacePlaceholders(template, resolver);
+ String maskedTemplate = template.replace("${", prefix);
+ PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper(prefix, "}");
+ String result = helper.replacePlaceholders(maskedTemplate, resolver);
+ result = result.replace(prefix, "${");
response.setContentType(getContentType());
response.getWriter().append(result);
}
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2012-2015 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.oauth2.provider.endpoint;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+/**
+ * @author Dave Syer
+ *
+ */
+public class SpelViewTests {
+
+ private SpelView view;
+
+ private MockHttpServletResponse response = new MockHttpServletResponse();
+ private MockHttpServletRequest request = new MockHttpServletRequest();
+
+ @Test
+ public void sunnyDay() throws Exception {
+ view = new SpelView("${hit}");
+ view.render(Collections.singletonMap("hit", "Ouch"), request, response);
+ assertEquals("Ouch", response.getContentAsString());
+ }
+
+ @Test
+ public void nonRecursive() throws Exception {
+ view = new SpelView("${hit}");
+ view.render(Collections.singletonMap("hit", "${ouch}"), request, response);
+ // Expressions embedded in resolved values do not resolve recursively
+ assertEquals("${ouch}", response.getContentAsString());
+ }
+
+ @Test
+ public void recursive() throws Exception {
+ // Recursive expressions in the template resolve
+ view = new SpelView("${${hit}}");
+ Map<String,Object> map = new HashMap<String, Object>();
+ map.put("hit", "me");
+ map.put("me", "${ouch}");
+ view.render(map, request, response);
+ // But expressions embedded in resolved values do not
+ assertEquals("${ouch}", response.getContentAsString());
+ }
+
+}

1 comment on commit fff77d3

@rudr4sarkar

Nice One ๐Ÿ‘

Please sign in to comment.