From fd7e0b405b1fa418ed7449200a42d77a145f92e8 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 22 Jul 2011 16:27:33 +0000 Subject: [PATCH] SPR-8515 Check for traversal to parent directory via ../ in resource requests --- .../resource/ResourceHttpRequestHandler.java | 12 +++++++++++- .../ResourceHttpRequestHandlerTests.java | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 8540de9c2e..18750ca2a0 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -18,6 +18,7 @@ package org.springframework.web.servlet.resource; import java.io.IOException; import java.util.List; + import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -142,7 +143,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE + "' is not set"); } - if (!StringUtils.hasText(path) || path.contains("WEB-INF") || path.contains("META-INF")) { + if (!StringUtils.hasText(path) || isInvalidPath(path)) { if (logger.isDebugEnabled()) { logger.debug("Ignoring invalid resource path [" + path + "]"); } @@ -172,6 +173,15 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H return null; } + /** + * Returns {@code true} if the given path is not a valid resource path. + * The default implementation rejects paths containing "WEB-INF" or "META-INF" as well as paths with + * relative paths ("../") that result in access of a parent directory. + */ + protected boolean isInvalidPath(String path) { + return (path.contains("WEB-INF") || path.contains("META-INF") || StringUtils.cleanPath(path).startsWith("..")); + } + /** * Determine an appropriate media type for the given resource. * @param resource the resource to check diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index c9af1a43a0..b579751be5 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -122,6 +122,22 @@ public class ResourceHttpRequestHandlerTests { assertEquals("function foo() { console.log(\"hello world\"); }", response.getContentAsString()); } + @Test + public void getResourceViaDirectoryTraversal() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + + request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "../testsecret/secret.txt"); + MockHttpServletResponse response = new MockHttpServletResponse(); + handler.handleRequest(request, response); + assertEquals(404, response.getStatus()); + + request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "test/../../testsecret/secret.txt"); + response = new MockHttpServletResponse(); + handler.handleRequest(request, response); + assertEquals(404, response.getStatus()); + } + @Test public void notModified() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest();