From 24e23d4fb6a72b6e5ec2259e556c2d6ebbdc11fb Mon Sep 17 00:00:00 2001 From: Jeremy Grelle Date: Thu, 5 Jun 2008 21:27:08 +0000 Subject: [PATCH] SWF-706 - ResourceServlet allows HTTP access to sensitive files --- .../js/resource/ResourceServlet.java | 47 ++++++++++++++----- .../js/resource/ResourceServletTests.java | 21 +++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/spring-js/src/main/java/org/springframework/js/resource/ResourceServlet.java b/spring-js/src/main/java/org/springframework/js/resource/ResourceServlet.java index d7ccf496..a94964bf 100644 --- a/spring-js/src/main/java/org/springframework/js/resource/ResourceServlet.java +++ b/spring-js/src/main/java/org/springframework/js/resource/ResourceServlet.java @@ -30,7 +30,6 @@ import java.util.zip.GZIPOutputStream; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -38,13 +37,14 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; +import org.springframework.web.servlet.HttpServletBean; /** * Special resource servlet for efficiently resolving and rendering static resources from within a JAR file. * * @author Jeremy Grelle */ -public class ResourceServlet extends HttpServlet { +public class ResourceServlet extends HttpServletBean { private static final String HTTP_CONTENT_LENGTH_HEADER = "Content-Length"; @@ -54,14 +54,16 @@ public class ResourceServlet extends HttpServlet { private static final String HTTP_CACHE_CONTROL_HEADER = "Cache-Control"; - private static final String GZIP_ENABLED_PARAM = "gzipEnabled"; - private static final Log log = LogFactory.getLog(ResourceServlet.class); private final String protectedPath = "/?WEB-INF/.*"; private boolean gzipEnabled = true; + private String[] allowedResourcePaths = new String[] { "/**/*.css", "/**/*.gif", "/**/*.ico", "/**/*.jpeg", + "/**/*.jpg", "/**/*.js", "/**/*.png", "META-INF/**/*.css", "META-INF/**/*.gif", "META-INF/**/*.ico", + "META-INF/**/*.jpeg", "META-INF/**/*.jpg", "META-INF/**/*.js", "META-INF/**/*.png" }; + private Map defaultMimeTypes = new HashMap(); { defaultMimeTypes.put(".css", "text/css"); @@ -79,13 +81,6 @@ public class ResourceServlet extends HttpServlet { compressedMimeTypes.add("text/javascript"); } - public void init() throws ServletException { - String gzipEnabledParamValue = getServletConfig().getInitParameter(GZIP_ENABLED_PARAM); - if (StringUtils.hasText(gzipEnabledParamValue)) { - gzipEnabled = Boolean.valueOf(gzipEnabledParamValue).booleanValue(); - } - } - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { String rawResourcePath = request.getPathInfo(); @@ -215,7 +210,7 @@ public class ResourceServlet extends HttpServlet { URL[] resources = new URL[localResourcePaths.length]; for (int i = 0; i < localResourcePaths.length; i++) { String localResourcePath = localResourcePaths[i]; - if (localResourcePath.matches(protectedPath)) { + if (!isAllowed(localResourcePath)) { if (log.isWarnEnabled()) { log.warn("An attempt to access a protected resource at " + localResourcePath + " was disallowed."); } @@ -224,6 +219,14 @@ public class ResourceServlet extends HttpServlet { URL resource = getServletContext().getResource(localResourcePath); if (resource == null) { String jarResourcePath = "META-INF" + localResourcePath; + if (!isAllowed(jarResourcePath)) { + if (log.isWarnEnabled()) { + log + .warn("An attempt to access a protected resource at " + jarResourcePath + + " was disallowed."); + } + return null; + } if (log.isDebugEnabled()) { log.debug("Searching classpath for resource: " + jarResourcePath); } @@ -241,6 +244,18 @@ public class ResourceServlet extends HttpServlet { return resources; } + private boolean isAllowed(String resourcePath) { + if (resourcePath.matches(protectedPath)) { + return false; + } + /* + * PathMatcher pathMatcher = new AntPathMatcher(); for (int i = 0; i < allowedResourcePaths.length; i++) { + * String pattern = allowedResourcePaths[i]; if (pathMatcher.match(pattern, resourcePath)) { return true; } } + * return false; + */ + return true; + } + /** * Set HTTP headers to allow caching for the given number of seconds. * @param seconds number of seconds into the future that the response should be cacheable for @@ -322,4 +337,12 @@ public class ResourceServlet extends HttpServlet { // noop } } + + public void setGzipEnabled(boolean gzipEnabled) { + this.gzipEnabled = gzipEnabled; + } + + public void setAllowedResourcePaths(String[] allowedResourcePaths) { + this.allowedResourcePaths = allowedResourcePaths; + } } diff --git a/spring-js/src/test/java/org/springframework/js/resource/ResourceServletTests.java b/spring-js/src/test/java/org/springframework/js/resource/ResourceServletTests.java index d5623263..4a60212a 100644 --- a/spring-js/src/test/java/org/springframework/js/resource/ResourceServletTests.java +++ b/spring-js/src/test/java/org/springframework/js/resource/ResourceServletTests.java @@ -67,6 +67,27 @@ public class ResourceServletTests extends TestCase { assertEquals(404, response.getStatus()); } + public final void testExecute_DisallowedPath() throws Exception { + String requestPath = "/persistence.xml"; + request.setPathInfo(requestPath); + servlet.doGet(request, response); + + assertEquals(404, response.getStatus()); + } + + public final void testBenchmark() throws Exception { + String requestPath = "/dojo/dojo.js"; + + // for (int i = 0; i < 100; i++) { + // request = new MockHttpServletRequest(); + // response = new MockHttpServletResponse(); + request.setPathInfo(requestPath); + servlet.doGet(request, response); + + assertEquals(200, response.getStatus()); + // } + } + private class ResourceTestMockServletContext extends MockServletContext { public String getMimeType(String filePath) {