diff --git a/core/src/main/java/org/apache/struts2/RequestUtils.java b/core/src/main/java/org/apache/struts2/RequestUtils.java index 5840ff06b..651a0a27f 100644 --- a/core/src/main/java/org/apache/struts2/RequestUtils.java +++ b/core/src/main/java/org/apache/struts2/RequestUtils.java @@ -53,6 +53,11 @@ public class RequestUtils { * Retrieves the current request servlet path. * Deals with differences between servlet specs (2.2 vs 2.3+) * + *

Note: the fallback branch extracts from the raw {@code requestURI}, which may + * retain percent-encoded characters. Callers must not apply additional URL decoding + * to the returned value because the servlet container has already performed decoding + * where applicable.

+ * * @param request the request * @return the servlet path */ diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java index 3b7a64fd2..f9f83a5fd 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java @@ -32,16 +32,14 @@ import org.apache.struts2.webjars.WebJarUrlProvider; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.UnsupportedEncodingException; +import java.util.Optional; import java.net.URL; -import java.net.URLDecoder; import java.util.ArrayList; import java.util.Calendar; import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.StringTokenizer; /** @@ -105,11 +103,6 @@ public class DefaultStaticContentLoader implements StaticContentLoader { */ protected final Calendar lastModifiedCal = Calendar.getInstance(); - /** - * Store state of StrutsConstants.STRUTS_I18N_ENCODING setting. - */ - protected String encoding; - protected boolean devMode; protected WebJarUrlProvider webJarUrlProvider; @@ -145,16 +138,6 @@ public class DefaultStaticContentLoader implements StaticContentLoader { this.serveStaticBrowserCache = BooleanUtils.toBoolean(serveStaticBrowserCache); } - /** - * Modify state of StrutsConstants.STRUTS_I18N_ENCODING setting. - * - * @param encoding New setting - */ - @Inject(StrutsConstants.STRUTS_I18N_ENCODING) - public void setEncoding(String encoding) { - this.encoding = encoding; - } - @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String devMode) { this.devMode = Boolean.parseBoolean(devMode); @@ -222,6 +205,14 @@ public class DefaultStaticContentLoader implements StaticContentLoader { throws IOException { String name = cleanupPath(path); + Optional canonical = Validator.canonicalisePath(name); + if (canonical.isEmpty()) { + LOG.debug("Rejecting static resource request: path escapes intended scope"); + sendNotFound(response); + return; + } + name = "/" + canonical.get(); + if (name.startsWith(WEBJARS_REQUEST_PREFIX)) { if (!findWebJarResource(name, path, request, response)) { sendNotFound(response); @@ -353,17 +344,12 @@ public class DefaultStaticContentLoader implements StaticContentLoader { * @param name resource name * @param packagePrefix The package prefix to use to locate the resource * @return full path - * @throws UnsupportedEncodingException If there is a encoding problem */ - protected String buildPath(String name, String packagePrefix) throws UnsupportedEncodingException { - String resourcePath; + protected String buildPath(String name, String packagePrefix) { if (packagePrefix.endsWith("/") && name.startsWith("/")) { - resourcePath = packagePrefix + name.substring(1); - } else { - resourcePath = packagePrefix + name; + return packagePrefix + name.substring(1); } - - return URLDecoder.decode(resourcePath, encoding); + return packagePrefix + name; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java b/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java index 149b18c81..31af4f4ad 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java @@ -27,7 +27,9 @@ import org.apache.struts2.config.StrutsBeanSelectionProvider; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; - +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Optional; /** * Interface for loading static resources, based on a path. After implementing your own static content loader * you must tell the framework how to use it, eg. @@ -99,5 +101,32 @@ public interface StaticContentLoader { return uiStaticContentPath; } } + /** + * Normalises a resource path by resolving {@code .} and {@code ..} + * segments and converting backslash separators to forward slashes. + * + *

Returns {@link Optional#empty()} if the resolved path would + * escape above the root (i.e. more {@code ..} segments than + * preceding path components).

+ * + * @param path the raw path to normalise (must not be null) + * @return the canonical path without leading slash, or empty if + * the path escapes above the root + */ + public static Optional canonicalisePath(String path) { + String normalised = path.replace('\\', '/'); + Deque segments = new ArrayDeque<>(); + for (String segment : normalised.split("/", -1)) { + if ("..".equals(segment)) { + if (segments.isEmpty()) { + return Optional.empty(); + } + segments.removeLast(); + } else if (!".".equals(segment) && !segment.isEmpty()) { + segments.addLast(segment); + } + } + return Optional.of(String.join("/", segments)); + } } } diff --git a/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java b/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java index 0b99f85b8..892ccd47f 100644 --- a/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java +++ b/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java @@ -123,15 +123,12 @@ public class DefaultWebJarUrlProvider implements WebJarUrlProvider { return Optional.empty(); } String normalized = StringUtils.stripStart(logicalPath, "/"); - if (normalized.contains("\\")) { + Optional canonical = StaticContentLoader.Validator.canonicalisePath(normalized); + if (canonical.isEmpty()) { + LOG.debug("Rejecting WebJar path that escapes above root: {}", logicalPath); return Optional.empty(); } - for (String segment : normalized.split("/")) { - if (segment.equals("..") || segment.equals(".")) { - LOG.debug("Rejecting WebJar path with traversal segment: {}", logicalPath); - return Optional.empty(); - } - } + normalized = canonical.get(); int slash = normalized.indexOf('/'); if (slash < 1 || slash == normalized.length() - 1) { return Optional.empty(); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java index db5caba14..d12164538 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java @@ -140,7 +140,24 @@ public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase { expect(hostConfigMock.getInitParameter("loggerFactory")).andStubReturn(null); defaultStaticContentLoader = new DefaultStaticContentLoader(); defaultStaticContentLoader.setHostConfig(hostConfigMock); - defaultStaticContentLoader.setEncoding("UTF-8"); defaultStaticContentLoader.setStaticContentPath("/static"); } + public void testBuildPathDoesNotDecodePercentEncoding() { + String result = defaultStaticContentLoader.buildPath("/%2e%2e/secret", "static/"); + assertEquals("static/%2e%2e/secret", result); + } + + public void testFindStaticResourceRejectsLiteralTraversal() throws Exception { + responseMock.sendError(HttpServletResponse.SC_NOT_FOUND); + expectLastCall(); + replay(responseMock); + defaultStaticContentLoader.findStaticResource("/static/../../../etc/passwd", requestMock, responseMock); + } + + public void testFindStaticResourceRejectsBackslashPath() throws Exception { + responseMock.sendError(HttpServletResponse.SC_NOT_FOUND); + expectLastCall(); + replay(responseMock); + defaultStaticContentLoader.findStaticResource("/static/..\\..\\secret", requestMock, responseMock); + } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java index 388aff42e..009721b08 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java @@ -68,7 +68,6 @@ public class DefaultStaticContentLoaderWebJarTest { webJarLoader.setServeStaticContent("true"); webJarLoader.setStaticContentPath("/static"); webJarLoader.setServeStaticBrowserCache("true"); - webJarLoader.setEncoding("UTF-8"); org.apache.struts2.webjars.DefaultWebJarUrlProvider provider = new org.apache.struts2.webjars.DefaultWebJarUrlProvider(); provider.setEnabled(String.valueOf(enabled)); @@ -116,4 +115,15 @@ public class DefaultStaticContentLoaderWebJarTest { verify(response).sendError(HttpServletResponse.SC_NOT_FOUND); } + + @Test + public void webJarEncodedTraversalReturns404() throws Exception { + DefaultStaticContentLoader webJarLoader = newLoader(true); + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + + webJarLoader.findStaticResource("/static/webjars/jquery/%2e%2e/%2e%2e/etc/passwd", request, response); + + verify(response).sendError(HttpServletResponse.SC_NOT_FOUND); + } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java b/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java index fbf2a044a..a7c41c8a6 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java @@ -106,8 +106,6 @@ public class StaticContentLoaderTest extends TestCase { hostConfigMock.expectAndReturn("getInitParameter", C.args(C.eq("packages")), null); hostConfigMock.expectAndReturn("getInitParameter", C.args(C.eq("loggerFactory")), null); - contentLoader.setEncoding("utf-8"); - contentLoader.setHostConfig((HostConfig) hostConfigMock.proxy()); }