Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Protect against RFD exploits

Browse Source 
Issue: SPR-13548
This commit is contained in:
Rossen Stoyanchev
2015-10-09 14:08:09 -04:00
committed by Stephane Nicoll
parent daada71c36
commit 03f547eb98
8 changed files with 196 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java
View File

@@ -128,6 +128,10 @@ public class ContentNegotiationManagerFactoryBean
this.useJaf = useJaf;
}
private boolean isUseJafTurnedOff() {
return (this.useJaf != null && !this.useJaf);
}
/**
* Indicate whether a request parameter should be used to determine the
* requested media type with the <em>2nd highest priority</em>, i.e.
@@ -184,7 +188,7 @@ public class ContentNegotiationManagerFactoryBean
if (this.favorPathExtension) {
PathExtensionContentNegotiationStrategy strategy;
if (this.servletContext != null) {
if (this.servletContext != null && !isUseJafTurnedOff()) {
strategy = new ServletPathExtensionContentNegotiationStrategy(this.servletContext, this.mediaTypes);
}
else {

4
spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java
View File

@@ -63,7 +63,7 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont
urlPathHelper.setUrlDecode(false);
}
private boolean useJaf = JAF_PRESENT;
private boolean useJaf = true;
/**
@@ -109,7 +109,7 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont
@Override
protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension) {
if (this.useJaf) {
if (this.useJaf && JAF_PRESENT) {
MediaType jafMediaType = JafMediaTypeFactory.getMediaType("file." + extension);
if (jafMediaType != null && !MediaType.APPLICATION_OCTET_STREAM.equals(jafMediaType)) {
return jafMediaType;

2
spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java
View File

@@ -405,7 +405,7 @@ public class UrlPathHelper {
* @see java.net.URLDecoder#decode(String)
*/
public String decodeRequestString(HttpServletRequest request, String source) {
if (this.urlDecode) {
if (this.urlDecode && source != null) {
return decodeInternal(request, source);
}
return source;

11
spring-web/src/main/java/org/springframework/web/util/WebUtils.java
View File

@@ -709,20 +709,23 @@ public abstract class WebUtils {
}
/**
* Extract the full URL filename (including file extension) from the given request URL path.
* Correctly resolves nested paths such as "/products/view.html" as well.
* Extract the full URL filename (including file extension) from the given
* request URL path. Correctly resolve nested paths such as
* "/products/view.html" and remove any path and or query parameters.
* @param urlPath the request URL path (e.g. "/products/index.html")
* @return the extracted URI filename (e.g. "index.html")
*/
public static String extractFullFilenameFromUrlPath(String urlPath) {
int end = urlPath.indexOf(';');
int end = urlPath.indexOf('?');
if (end == -1) {
end = urlPath.indexOf('?');
end = urlPath.indexOf('#');
if (end == -1) {
end = urlPath.length();
}
}
int begin = urlPath.lastIndexOf('/', end) + 1;
int paramIndex = urlPath.indexOf(';', begin);
end = (paramIndex != -1 && paramIndex < end ? paramIndex : end);
return urlPath.substring(begin, end);
}

58
spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java
View File

@@ -26,6 +26,8 @@ import org.junit.Before;
import org.junit.Test;
import org.springframework.http.MediaType;
import org.springframework.mock.web.test.MockHttpServletRequest;
import org.springframework.mock.web.test.MockServletContext;
import org.springframework.util.StringUtils;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.ServletWebRequest;
@@ -43,7 +45,10 @@ public class ContentNegotiationManagerFactoryBeanTests {
@Before
public void setup() {
this.servletRequest = new MockHttpServletRequest();
TestServletContext servletContext = new TestServletContext();
servletContext.getMimeTypes().put("foo", "application/foo");
this.servletRequest = new MockHttpServletRequest(servletContext);
this.webRequest = new ServletWebRequest(this.servletRequest);
this.factoryBean = new ContentNegotiationManagerFactoryBean();
@@ -74,16 +79,36 @@ public class ContentNegotiationManagerFactoryBeanTests {
}
@Test
public void addMediaTypes() throws Exception {
Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
mediaTypes.put("json", MediaType.APPLICATION_JSON);
this.factoryBean.addMediaTypes(mediaTypes);
public void favorPath() throws Exception {
this.factoryBean.setFavorPathExtension(true);
this.factoryBean.addMediaTypes(Collections.singletonMap("bar", new MediaType("application", "bar")));
this.factoryBean.afterPropertiesSet();
ContentNegotiationManager manager = this.factoryBean.getObject();
this.servletRequest.setRequestURI("/flower.json");
assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
this.servletRequest.setRequestURI("/flower.foo");
assertEquals(Collections.singletonList(new MediaType("application", "foo")),
manager.resolveMediaTypes(this.webRequest));
this.servletRequest.setRequestURI("/flower.bar");
assertEquals(Collections.singletonList(new MediaType("application", "bar")),
manager.resolveMediaTypes(this.webRequest));
this.servletRequest.setRequestURI("/flower.gif");
assertEquals(Collections.singletonList(MediaType.IMAGE_GIF), manager.resolveMediaTypes(this.webRequest));
}
@Test
public void favorPathWithJafTurnedOff() throws Exception {
this.factoryBean.setFavorPathExtension(true);
this.factoryBean.setUseJaf(false);
this.factoryBean.afterPropertiesSet();
ContentNegotiationManager manager = this.factoryBean.getObject();
this.servletRequest.setRequestURI("/flower.foo");
assertEquals(Collections.emptyList(), manager.resolveMediaTypes(this.webRequest));
this.servletRequest.setRequestURI("/flower.gif");
assertEquals(Collections.emptyList(), manager.resolveMediaTypes(this.webRequest));
}
@Test
@@ -130,4 +155,21 @@ public class ContentNegotiationManagerFactoryBeanTests {
assertEquals(Arrays.asList(MediaType.APPLICATION_JSON), manager.resolveMediaTypes(this.webRequest));
}
private static class TestServletContext extends MockServletContext {
private final Map<String, String> mimeTypes = new HashMap<>();
public Map<String, String> getMimeTypes() {
return this.mimeTypes;
}
@Override
public String getMimeType(String filePath) {
String extension = StringUtils.getFilenameExtension(filePath);
return getMimeTypes().get(extension);
}
}
}

8
spring-web/src/test/java/org/springframework/web/util/WebUtilsTests.java
View File

@@ -62,9 +62,17 @@ public class WebUtilsTests {
assertEquals("index.html", WebUtils.extractFullFilenameFromUrlPath("index.html"));
assertEquals("index.html", WebUtils.extractFullFilenameFromUrlPath("/index.html"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html#/a"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html#/path/a"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html#/path/a.do"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=a"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=/path/a"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=/path/a.do"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=/path/a#/path/a"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products/view.html?param=/path/a.do#/path/a.do"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products;q=11/view.html?param=/path/a.do"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products;q=11/view.html;r=22?param=/path/a.do"));
assertEquals("view.html", WebUtils.extractFullFilenameFromUrlPath("/products;q=11/view.html;r=22;s=33?param=/path/a.do"));
}
@Test

69
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java
View File

@@ -18,26 +18,32 @@ package org.springframework.web.servlet.mvc.method.annotation;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpOutputMessage;
import org.springframework.http.MediaType;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.server.ServletServerHttpRequest;
import org.springframework.http.server.ServletServerHttpResponse;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.accept.ContentNegotiationManager;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.util.UrlPathHelper;
/**
* Extends {@link AbstractMessageConverterMethodArgumentResolver} with the ability to handle
@@ -52,8 +58,24 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
private static final MediaType MEDIA_TYPE_APPLICATION = new MediaType("application");
private static final UrlPathHelper RAW_URL_PATH_HELPER = new UrlPathHelper();
private static final UrlPathHelper DECODING_URL_PATH_HELPER = new UrlPathHelper();
static {
RAW_URL_PATH_HELPER.setRemoveSemicolonContent(false);
RAW_URL_PATH_HELPER.setUrlDecode(false);
}
/* Extensions associated with the built-in message converters */
private static final Set<String> WHITELISTED_EXTENSIONS = new HashSet<String>(Arrays.asList(
"txt", "text", "json", "xml", "atom", "rss", "png", "jpe", "jpeg", "jpg", "gif", "wbmp", "bmp"));
private final ContentNegotiationManager contentNegotiationManager;
private final Set<String> safeExtensions = new HashSet<String>();
protected AbstractMessageConverterMethodProcessor(List<HttpMessageConverter<?>> messageConverters) {
this(messageConverters, null);
@@ -64,6 +86,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
super(messageConverters);
this.contentNegotiationManager = (manager != null ? manager : new ContentNegotiationManager());
this.safeExtensions.addAll(this.contentNegotiationManager.getAllFileExtensions());
this.safeExtensions.addAll(WHITELISTED_EXTENSIONS);
}
@@ -140,6 +164,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
selectedMediaType = selectedMediaType.removeQualityValue();
for (HttpMessageConverter<?> messageConverter : this.messageConverters) {
if (messageConverter.canWrite(returnValueClass, selectedMediaType)) {
addContentDispositionHeader(inputMessage, outputMessage);
((HttpMessageConverter<T>) messageConverter).write(returnValue, selectedMediaType, outputMessage);
if (logger.isDebugEnabled()) {
logger.debug("Written [" + returnValue + "] as \"" + selectedMediaType + "\" using [" +
@@ -194,4 +219,48 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
return (MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceTypeToUse) <= 0 ? acceptType : produceTypeToUse);
}
/**
* Check if the path has a file extension and whether the extension is either
* {@link #WHITELISTED_EXTENSIONS whitelisted} or
* {@link ContentNegotiationManager#getAllFileExtensions() explicitly
* registered}. If not add a 'Content-Disposition' header with a safe
* attachment file name ("f.txt") to prevent RFD exploits.
*/
private void addContentDispositionHeader(ServletServerHttpRequest request,
ServletServerHttpResponse response) {
HttpHeaders headers = response.getHeaders();
if (headers.containsKey("Content-Disposition")) {
return;
}
HttpServletRequest servletRequest = request.getServletRequest();
String requestUri = RAW_URL_PATH_HELPER.getOriginatingRequestUri(servletRequest);
int index = requestUri.lastIndexOf('/') + 1;
String filename = requestUri.substring(index);
String pathParams = "";
index = filename.indexOf(';');
if (index != -1) {
pathParams = filename.substring(index);
filename = filename.substring(0, index);
}
filename = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, filename);
String ext = StringUtils.getFilenameExtension(filename);
pathParams = DECODING_URL_PATH_HELPER.decodeRequestString(servletRequest, pathParams);
String extInPathParams = StringUtils.getFilenameExtension(pathParams);
if (!isSafeExtension(ext) || !isSafeExtension(extInPathParams)) {
headers.add("Content-Disposition", "attachment;filename=f.txt");
}
}
private boolean isSafeExtension(String extension) {
return (!StringUtils.hasText(extension) ||
this.safeExtensions.contains(extension.toLowerCase(Locale.ENGLISH)));
}
}

54
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java
View File

@@ -19,6 +19,7 @@ package org.springframework.web.servlet.mvc.method.annotation;
import java.io.Serializable;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.junit.Before;
@@ -35,6 +36,7 @@ import org.springframework.mock.web.test.MockHttpServletRequest;
import org.springframework.mock.web.test.MockHttpServletResponse;
import org.springframework.util.MultiValueMap;
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
import org.springframework.web.bind.WebDataBinder;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.support.WebDataBinderFactory;
@@ -42,6 +44,7 @@ import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.method.support.ModelAndViewContainer;
import org.springframework.web.util.WebUtils;
import static org.junit.Assert.*;
@@ -224,6 +227,57 @@ public class RequestResponseBodyMethodProcessorTests {
assertEquals("text/plain;charset=UTF-8", servletResponse.getHeader("Content-Type"));
}
@Test
public void addContentDispositionHeader() throws Exception {
ContentNegotiationManagerFactoryBean factory = new ContentNegotiationManagerFactoryBean();
factory.addMediaType("pdf", new MediaType("application", "pdf"));
factory.afterPropertiesSet();
RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(
Collections.<HttpMessageConverter<?>>singletonList(new StringHttpMessageConverter()),
factory.getObject());
assertContentDisposition(processor, false, "/hello.json", "whitelisted extension");
assertContentDisposition(processor, false, "/hello.pdf", "registered extension");
assertContentDisposition(processor, true, "/hello.dataless", "uknown extension");
// path parameters
assertContentDisposition(processor, false, "/hello.json;a=b", "path param shouldn't cause issue");
assertContentDisposition(processor, true, "/hello.json;a=b;setup.dataless", "uknown ext in path params");
assertContentDisposition(processor, true, "/hello.dataless;a=b;setup.json", "uknown ext in filename");
assertContentDisposition(processor, false, "/hello.json;a=b;setup.json", "whitelisted extensions");
// encoded dot
assertContentDisposition(processor, true, "/hello%2Edataless;a=b;setup.json", "encoded dot in filename");
assertContentDisposition(processor, true, "/hello.json;a=b;setup%2Edataless", "encoded dot in path params");
assertContentDisposition(processor, true, "/hello.dataless%3Bsetup.bat", "encoded dot in path params");
this.servletRequest.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/hello.bat");
assertContentDisposition(processor, true, "/bonjour", "forwarded URL");
this.servletRequest.removeAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE);
}
private void assertContentDisposition(RequestResponseBodyMethodProcessor processor,
boolean expectContentDisposition, String requestURI, String comment) throws Exception {
this.servletRequest.setRequestURI(requestURI);
processor.handleReturnValue("body", this.returnTypeString, this.mavContainer, this.webRequest);
String header = servletResponse.getHeader("Content-Disposition");
if (expectContentDisposition) {
assertEquals("Expected 'Content-Disposition' header. Use case: '" + comment + "'",
"attachment;filename=f.txt", header);
}
else {
assertNull("Did not expect 'Content-Disposition' header. Use case: '" + comment + "'", header);
}
this.servletRequest = new MockHttpServletRequest();
this.servletResponse = new MockHttpServletResponse();
this.webRequest = new ServletWebRequest(servletRequest, servletResponse);
}
public String handle(
@RequestBody List<SimpleBean> list,