diff --git a/webconsole/src/main/java/org/apache/felix/webconsole/SimpleWebConsolePlugin.java b/webconsole/src/main/java/org/apache/felix/webconsole/SimpleWebConsolePlugin.java index e2a44b227b..ff0cbeed7f 100644 --- a/webconsole/src/main/java/org/apache/felix/webconsole/SimpleWebConsolePlugin.java +++ b/webconsole/src/main/java/org/apache/felix/webconsole/SimpleWebConsolePlugin.java @@ -20,6 +20,8 @@ import java.net.URL; +import java.util.ArrayDeque; +import java.util.Deque; import java.util.Dictionary; import java.util.HashMap; import java.util.Hashtable; @@ -202,6 +204,9 @@ protected final String[] getCssReferences() * resources are accessed like /system/console/abc/res/logo.gif, * the code here will try load resource /res/logo.gif from the * bundle, providing the plugin. + * Path segments are normalized before the resource is loaded, so a request + * cannot escape the plugin's resource directory using .. + * segments. * * * @param path the path to read. @@ -209,12 +214,43 @@ protected final String[] getCssReferences() */ protected URL getResource( String path ) { - return ( path != null && path.startsWith( labelRes ) ) ? // - getClass().getResource( path.substring( labelResLen ) ) + final String normalizedPath = path == null ? null : normalizePath(path); + return ( normalizedPath != null && normalizedPath.startsWith( labelRes ) ) ? // + getClass().getResource( normalizedPath.substring( labelResLen ) ) : null; } + /** + * Normalizes an absolute resource path without applying host filesystem + * semantics. Resource names always use forward slashes, irrespective of + * the operating system hosting the web console. + */ + private static String normalizePath(final String path) + { + final Deque segments = new ArrayDeque<>(); + for (final String segment : path.substring(1).split("/")) + { + if (segment.isEmpty() || ".".equals(segment)) + { + continue; + } + if ("..".equals(segment)) + { + if (!segments.isEmpty()) + { + segments.removeLast(); + } + } + else + { + segments.addLast(segment); + } + } + return '/' + String.join("/", segments); + } + + // -- begin methods for plugin registration/unregistration /** * This is an utility method. It is used to register the plugin service. Don't diff --git a/webconsole/src/main/java/org/apache/felix/webconsole/servlet/AbstractServlet.java b/webconsole/src/main/java/org/apache/felix/webconsole/servlet/AbstractServlet.java index 9bf9853453..61668f7128 100644 --- a/webconsole/src/main/java/org/apache/felix/webconsole/servlet/AbstractServlet.java +++ b/webconsole/src/main/java/org/apache/felix/webconsole/servlet/AbstractServlet.java @@ -25,6 +25,8 @@ import java.io.OutputStream; import java.net.URL; import java.net.URLConnection; +import java.util.ArrayDeque; +import java.util.Deque; import org.apache.felix.webconsole.internal.Util; @@ -74,19 +76,47 @@ public abstract class AbstractServlet extends HttpServlet { * Called to identify resources. * By default, if the path starts with "/res/" this is treated as a resource * and the URL to the resource is tried to be loaded via the class loader. + * Path segments are normalized before this check, so a request cannot escape + * the {@code /res/} directory using {@code ..} segments. * @param path the path * @return the URL of the resource or null if not found. */ protected URL getResource( final String path ) { + if (path == null) { + return null; + } final int index = path.indexOf( '/', 1 ); if (index != -1) { - if (path.substring(index).startsWith("/res/") ) { - return getClass().getResource( path.substring(index) ); + final String resourcePath = normalizePath(path.substring(index)); + if (resourcePath.startsWith("/res/") ) { + return getClass().getResource(resourcePath); } } return null; } + /** + * Normalizes an absolute resource path without applying host filesystem + * semantics. Resource names always use forward slashes, irrespective of + * the operating system hosting the web console. + */ + private static String normalizePath(final String path) { + final Deque segments = new ArrayDeque<>(); + for (final String segment : path.substring(1).split("/")) { + if (segment.isEmpty() || ".".equals(segment)) { + continue; + } + if ("..".equals(segment)) { + if (!segments.isEmpty()) { + segments.removeLast(); + } + } else { + segments.addLast(segment); + } + } + return '/' + String.join("/", segments); + } + /** * Handle get requests. This method can be used to return resources, like JSON responses etc. * If the plugin is serving a resource, this method call {@link HttpServletResponse#setStatus(int)}. diff --git a/webconsole/src/test/java/org/apache/felix/webconsole/AbstractWebConsolePluginTest.java b/webconsole/src/test/java/org/apache/felix/webconsole/AbstractWebConsolePluginTest.java index d4f811a020..0a02b3af04 100644 --- a/webconsole/src/test/java/org/apache/felix/webconsole/AbstractWebConsolePluginTest.java +++ b/webconsole/src/test/java/org/apache/felix/webconsole/AbstractWebConsolePluginTest.java @@ -120,6 +120,15 @@ public void test_getGetResourceMethod_extension_class() throws Exception assertNull( getGetResourceMethod.invoke( testPrivate, (Object[]) null ) ); } + + public void test_getResource_normalizesPath() { + final SimpleTestPlugin plugin = new SimpleTestPlugin(); + + assertNotNull(plugin.getResource("/test/res/ui/webconsole.css")); + assertNotNull(plugin.getResource("/test/res/ui/../ui/webconsole.css")); + assertNull(plugin.getResource("/test/res/../../META-INF/MANIFEST.MF")); + } + private static class PrivateTestPlugin extends TestPlugin { @SuppressWarnings("unused") @@ -175,4 +184,18 @@ protected void renderContent( HttpServletRequest req, HttpServletResponse res ) } } + + private static class SimpleTestPlugin extends SimpleWebConsolePlugin + { + SimpleTestPlugin() + { + super("test", "Test", null); + } + + @Override + protected void renderContent(final HttpServletRequest req, final HttpServletResponse res) + { + // nothing to render + } + } } diff --git a/webconsole/src/test/java/org/apache/felix/webconsole/servlet/AbstractServletTest.java b/webconsole/src/test/java/org/apache/felix/webconsole/servlet/AbstractServletTest.java new file mode 100644 index 0000000000..d9fab3336a --- /dev/null +++ b/webconsole/src/test/java/org/apache/felix/webconsole/servlet/AbstractServletTest.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.webconsole.servlet; + +import java.net.URL; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import junit.framework.TestCase; + +public class AbstractServletTest extends TestCase { + + public void testGetResourceNormalizesPath() { + final TestServlet servlet = new TestServlet(); + + assertNotNull(servlet.getResource("/test/res/ui/webconsole.css")); + assertNotNull(servlet.getResource("/test/res/ui/../ui/webconsole.css")); + assertNull(servlet.getResource("/test/res/../../META-INF/MANIFEST.MF")); + } + + private static final class TestServlet extends AbstractServlet { + + @Override + public void renderContent(final HttpServletRequest request, final HttpServletResponse response) + throws ServletException { + // nothing to render + } + + @Override + protected URL getResource(final String path) { + return super.getResource(path); + } + } +}