From: Markus Koschany Date: Sat, 26 Mar 2016 14:56:58 +0100 Subject: CVE-2015-5345 The Mapper component in Apache Tomcat processes redirects before considering security constraints and Filters, which allows remote attackers to determine the existence of a directory via a URL that lacks a trailing / (slash) character. Origin: https://svn.apache.org/viewvc?view=revision&revision=1715213 Origin: https://svn.apache.org/viewvc?view=revision&revision=1717212 --- java/org/apache/catalina/Context.java | 40 +++++++++++++++++++++ .../catalina/authenticator/FormAuthenticator.java | 14 ++++++++ java/org/apache/catalina/core/StandardContext.java | 41 ++++++++++++++++++++-- .../apache/catalina/core/mbeans-descriptors.xml | 8 +++++ .../apache/catalina/servlets/DefaultServlet.java | 28 ++++++++++++++- .../apache/catalina/servlets/WebdavServlet.java | 5 +++ .../org/apache/catalina/startup/FailedContext.java | 19 +++++++++- .../org/apache/tomcat/util/http/mapper/Mapper.java | 21 ++++++----- .../apache/catalina/startup/TomcatBaseTest.java | 3 +- webapps/docs/changelog.xml | 10 ++++++ webapps/docs/config/context.xml | 16 +++++++++ 11 files changed, 188 insertions(+), 17 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index 3eee519..b3e5a7b 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -1419,5 +1419,45 @@ public interface Context extends Container { * part of a redirect response. */ public boolean getSendRedirectBody(); + + /** + * If enabled, requests for a web application context root will be + * redirected (adding a trailing slash) by the Mapper. This is more + * efficient but has the side effect of confirming that the context path is + * valid. + * + * @param mapperContextRootRedirectEnabled Should the redirects be enabled? + */ + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled); + + /** + * Determines if requests for a web application context root will be + * redirected (adding a trailing slash) by the Mapper. This is more + * efficient but has the side effect of confirming that the context path is + * valid. + * + * @return {@code true} if the Mapper level redirect is enabled for this + * Context. + */ + public boolean getMapperContextRootRedirectEnabled(); + + /** + * If enabled, requests for a directory will be redirected (adding a + * trailing slash) by the Mapper. This is more efficient but has the + * side effect of confirming that the directory is valid. + * + * @param mapperDirectoryRedirectEnabled Should the redirects be enabled? + */ + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled); + + /** + * Determines if requests for a directory will be redirected (adding a + * trailing slash) by the Mapper. This is more efficient but has the + * side effect of confirming that the directory is valid. + * + * @return {@code true} if the Mapper level redirect is enabled for this + * Context. + */ + public boolean getMapperDirectoryRedirectEnabled(); } diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java index 7a728c8..f71e508 100644 --- a/java/org/apache/catalina/authenticator/FormAuthenticator.java +++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java @@ -265,6 +265,20 @@ public class FormAuthenticator // No -- Save this request and redirect to the form login page if (!loginAction) { + // If this request was to the root of the context without a trailing + // '/', need to redirect to add it else the submit of the login form + // may not go to the correct web application + if (request.getServletPath().length() == 0 && request.getPathInfo() == null) { + StringBuilder location = new StringBuilder(requestURI); + location.append('/'); + if (request.getQueryString() != null) { + location.append('?'); + location.append(request.getQueryString()); + } + response.sendRedirect(response.encodeRedirectURL(location.toString())); + return false; + } + session = request.getSessionInternal(true); if (log.isDebugEnabled()) { log.debug("Save request in session '" + session.getIdInternal() + "'"); diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index d5f5cc6..933e90b 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -863,8 +863,45 @@ public class StandardContext extends ContainerBase private boolean jndiExceptionOnFailedWrite = true; + boolean mapperContextRootRedirectEnabled = false; + + boolean mapperDirectoryRedirectEnabled = false; + // ----------------------------------------------------- Context Properties - + + @Override + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) { + this.mapperContextRootRedirectEnabled = mapperContextRootRedirectEnabled; + } + + + /** + * {@inheritDoc} + *

+ * The default value for this implementation is {@code false}. + */ + @Override + public boolean getMapperContextRootRedirectEnabled() { + return mapperContextRootRedirectEnabled; + } + + + @Override + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) { + this.mapperDirectoryRedirectEnabled = mapperDirectoryRedirectEnabled; + } + + + /** + * {@inheritDoc} + *

+ * The default value for this implementation is {@code false}. + */ + @Override + public boolean getMapperDirectoryRedirectEnabled() { + return mapperDirectoryRedirectEnabled; + } + @Override public boolean getSendRedirectBody() { return sendRedirectBody; @@ -1042,7 +1079,7 @@ public class StandardContext extends ContainerBase this.instanceManager = instanceManager; } - + @Override public String getEncodedPath() { return encodedPath; diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml b/java/org/apache/catalina/core/mbeans-descriptors.xml index 190d50f..d95ff2d 100644 --- a/java/org/apache/catalina/core/mbeans-descriptors.xml +++ b/java/org/apache/catalina/core/mbeans-descriptors.xml @@ -221,6 +221,14 @@ description="The object used for mapping" type="java.lang.Object"/> + + + + diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 7365f6b..cc1ab4d 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -366,6 +366,10 @@ public class DefaultServlet * @param request The servlet request we are processing */ protected String getRelativePath(HttpServletRequest request) { + return getRelativePath(request, false); + } + + protected String getRelativePath(HttpServletRequest request, boolean allowEmptyPath) { // IMPORTANT: DefaultServlet can be mapped to '/' or '/path/*' but always // serves resources from the web app root with context rooted paths. // i.e. it can not be used to mount the web app root under a sub-path @@ -775,7 +779,8 @@ public class DefaultServlet boolean serveContent = content; // Identify the requested resource path - String path = getRelativePath(request); + String path = getRelativePath(request, true); + if (debug > 0) { if (serveContent) log("DefaultServlet.serveResource: Serving resource '" + @@ -785,6 +790,12 @@ public class DefaultServlet path + "' headers only"); } + if (path.length() == 0) { + // Context root redirect + doDirectoryRedirect(request, response); + return; + } + CacheEntry cacheEntry = resources.lookupCache(path); if (!cacheEntry.exists) { @@ -853,6 +864,11 @@ public class DefaultServlet if (cacheEntry.context != null) { + if (!path.endsWith("/")) { + doDirectoryRedirect(request, response); + return; + } + // Skip directory listings if we have been configured to // suppress them if (!listings) { @@ -1060,6 +1076,16 @@ public class DefaultServlet } + private void doDirectoryRedirect(HttpServletRequest request, HttpServletResponse response) + throws IOException { + StringBuilder location = new StringBuilder(request.getRequestURI()); + location.append('/'); + if (request.getQueryString() != null) { + location.append('?'); + location.append(request.getQueryString()); + } + response.sendRedirect(response.encodeRedirectURL(location.toString())); + } /** * Parse the content-range header. diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java index 358b919..a7478d3 100644 --- a/java/org/apache/catalina/servlets/WebdavServlet.java +++ b/java/org/apache/catalina/servlets/WebdavServlet.java @@ -429,6 +429,11 @@ public class WebdavServlet */ @Override protected String getRelativePath(HttpServletRequest request) { + return getRelativePath(request, false); + } + + @Override + protected String getRelativePath(HttpServletRequest request, boolean allowEmptyPath) { // Are we being processed by a RequestDispatcher.include()? if (request.getAttribute( RequestDispatcher.INCLUDE_REQUEST_URI) != null) { diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java index 409783b..69fcd8a 100644 --- a/java/org/apache/catalina/startup/FailedContext.java +++ b/java/org/apache/catalina/startup/FailedContext.java @@ -652,4 +652,21 @@ public class FailedContext extends LifecycleMBeanBase implements Context { @Override public Object getMappingObject() { return null; } -} \ No newline at end of file + + @Override + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperContextRootRedirectEnabled() { return false; } + + @Override + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperDirectoryRedirectEnabled() { return false; } + +} diff --git a/java/org/apache/tomcat/util/http/mapper/Mapper.java b/java/org/apache/tomcat/util/http/mapper/Mapper.java index 6100a2b..30c7814 100644 --- a/java/org/apache/tomcat/util/http/mapper/Mapper.java +++ b/java/org/apache/tomcat/util/http/mapper/Mapper.java @@ -827,20 +827,13 @@ public final class Mapper { int pathOffset = path.getOffset(); int pathEnd = path.getEnd(); - int servletPath = pathOffset; boolean noServletPath = false; int length = contextVersion.path.length(); - if (length != (pathEnd - pathOffset)) { - servletPath = pathOffset + length; - } else { + if (length == (pathEnd - pathOffset)) { noServletPath = true; - path.append('/'); - pathOffset = path.getOffset(); - pathEnd = path.getEnd(); - servletPath = pathOffset+length; } - + int servletPath = pathOffset + length; path.setOffset(servletPath); // Rule 1 -- Exact Match @@ -877,8 +870,10 @@ public final class Mapper { if(mappingData.wrapper == null && noServletPath) { // The path is empty, redirect to "/" + path.append('/'); + pathEnd = path.getEnd(); mappingData.redirectPath.setChars - (path.getBuffer(), pathOffset, pathEnd-pathOffset); + (path.getBuffer(), pathOffset, pathEnd - pathOffset); path.setEnd(pathEnd - 1); return; } @@ -999,7 +994,11 @@ public final class Mapper { Object file = null; String pathStr = path.toString(); try { - file = contextVersion.resources.lookup(pathStr); + if (pathStr.length() == 0) { + file = contextVersion.resources.lookup("/"); + } else { + file = contextVersion.resources.lookup(pathStr); + } } catch(NamingException nex) { // Swallow, since someone else handles the 404 } diff --git a/test/org/apache/catalina/startup/TomcatBaseTest.java b/test/org/apache/catalina/startup/TomcatBaseTest.java index 33d5fd1..150b4f4 100644 --- a/test/org/apache/catalina/startup/TomcatBaseTest.java +++ b/test/org/apache/catalina/startup/TomcatBaseTest.java @@ -211,8 +211,7 @@ public abstract class TomcatBaseTest extends LoggingBaseTest { Map> resHead) throws IOException { URL url = new URL(path); - HttpURLConnection connection = - (HttpURLConnection) url.openConnection(); + HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setUseCaches(false); connection.setReadTimeout(readTimeout); if (reqHead != null) { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index e73d7b3..e565198 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -266,6 +266,16 @@ Only create XML parsing objects if required and fix associated potential memory leak in the default Servlet. (markt) + + Move the functionality that provides redirects for context roots and + directories where a trailing / is added from the Mapper to + the DefaultServlet. This enables such requests to be + processed by any configured Valves and Filters before the redirect is + made. This behaviour is configurable via the + mapperContextRootRedirectEnabled and + mapperDirectoryRedirectEnabled attributes of the Context + which may be used to restore the previous behaviour. (markt) + diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 6a16709..a961cf8 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -293,6 +293,22 @@ default value of false is used.

+ +

If enabled, requests for a web application context root will be + redirected (adding a trailing slash) if necessary by the Mapper rather + than the default Servlet. This is more efficient but has the side effect + of confirming that the context path exists. If not specified, the + default value of false is used.

+ + + +

If enabled, requests for a web application directory will be + redirected (adding a trailing slash) if necessary by the Mapper rather + than the default Servlet. This is more efficient but has the side effect + of confirming that the directory is exists. If not specified, the + default value of false is used.

+ +

Set to true to ignore any settings in both the global or Host default contexts. By default, settings