Skip to content
This repository was archived by the owner on May 31, 2022. It is now read-only.

Commit fff77d3

Browse files
author
Dave Syer
committed
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).
1 parent 4f01e4a commit fff77d3

File tree

2 files changed

+75
-4
lines changed
  • spring-security-oauth2/src
    • main/java/org/springframework/security/oauth2/provider/endpoint
    • test/java/org/springframework/security/oauth2/provider/endpoint

2 files changed

+75
-4
lines changed

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/endpoint/SpelView.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.springframework.expression.Expression;
2424
import org.springframework.expression.spel.standard.SpelExpressionParser;
2525
import org.springframework.expression.spel.support.StandardEvaluationContext;
26+
import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
2627
import org.springframework.util.PropertyPlaceholderHelper;
2728
import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver;
2829
import org.springframework.web.servlet.View;
@@ -35,19 +36,19 @@
3536
class SpelView implements View {
3637

3738
private final String template;
39+
40+
private final String prefix;
3841

3942
private final SpelExpressionParser parser = new SpelExpressionParser();
4043

4144
private final StandardEvaluationContext context = new StandardEvaluationContext();
4245

43-
private PropertyPlaceholderHelper helper;
44-
4546
private PlaceholderResolver resolver;
4647

4748
public SpelView(String template) {
4849
this.template = template;
50+
this.prefix = new RandomValueStringGenerator().generate() + "{";
4951
this.context.addPropertyAccessor(new MapAccessor());
50-
this.helper = new PropertyPlaceholderHelper("${", "}");
5152
this.resolver = new PlaceholderResolver() {
5253
public String resolvePlaceholder(String name) {
5354
Expression expression = parser.parseExpression(name);
@@ -68,7 +69,10 @@ public void render(Map<String, ?> model, HttpServletRequest request, HttpServlet
6869
.getPath();
6970
map.put("path", (Object) path==null ? "" : path);
7071
context.setRootObject(map);
71-
String result = helper.replacePlaceholders(template, resolver);
72+
String maskedTemplate = template.replace("${", prefix);
73+
PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper(prefix, "}");
74+
String result = helper.replacePlaceholders(maskedTemplate, resolver);
75+
result = result.replace(prefix, "${");
7276
response.setContentType(getContentType());
7377
response.getWriter().append(result);
7478
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright 2012-2015 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.oauth2.provider.endpoint;
18+
19+
import static org.junit.Assert.assertEquals;
20+
21+
import java.util.Collections;
22+
import java.util.HashMap;
23+
import java.util.Map;
24+
25+
import org.junit.Test;
26+
import org.springframework.mock.web.MockHttpServletRequest;
27+
import org.springframework.mock.web.MockHttpServletResponse;
28+
29+
/**
30+
* @author Dave Syer
31+
*
32+
*/
33+
public class SpelViewTests {
34+
35+
private SpelView view;
36+
37+
private MockHttpServletResponse response = new MockHttpServletResponse();
38+
private MockHttpServletRequest request = new MockHttpServletRequest();
39+
40+
@Test
41+
public void sunnyDay() throws Exception {
42+
view = new SpelView("${hit}");
43+
view.render(Collections.singletonMap("hit", "Ouch"), request, response);
44+
assertEquals("Ouch", response.getContentAsString());
45+
}
46+
47+
@Test
48+
public void nonRecursive() throws Exception {
49+
view = new SpelView("${hit}");
50+
view.render(Collections.singletonMap("hit", "${ouch}"), request, response);
51+
// Expressions embedded in resolved values do not resolve recursively
52+
assertEquals("${ouch}", response.getContentAsString());
53+
}
54+
55+
@Test
56+
public void recursive() throws Exception {
57+
// Recursive expressions in the template resolve
58+
view = new SpelView("${${hit}}");
59+
Map<String,Object> map = new HashMap<String, Object>();
60+
map.put("hit", "me");
61+
map.put("me", "${ouch}");
62+
view.render(map, request, response);
63+
// But expressions embedded in resolved values do not
64+
assertEquals("${ouch}", response.getContentAsString());
65+
}
66+
67+
}

0 commit comments

Comments
 (0)