Skip to content

Commit 8437193

Browse files
markt-asfmarkt-asf
committed
Move the functionality that provides redirects for context roots and directories where a trailing <code>/</code> 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.
This is part 1 of 3 of the fix for CVE-2015-5345 git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1715206 13f79535-47bb-0310-9956-ffa450edef68
1 parent 9cc01c8 commit 8437193

File tree

12 files changed

+232
-8
lines changed

12 files changed

+232
-8
lines changed

‎java/org/apache/catalina/Context.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,4 +1714,44 @@ Set<String> addServletSecurity(ServletRegistration.Dynamic registration,
17141714
* false}
17151715
*/
17161716
public boolean getValidateClientProvidedNewSessionId();
1717+
1718+
/**
1719+
* If enabled, requests for a web application context root will be
1720+
* redirected (adding a trailing slash) by the Mapper. This is more
1721+
* efficient but has the side effect of confirming that the context path is
1722+
* valid.
1723+
*
1724+
* @param mapperContextRootRedirectEnabled Should the redirects be enabled?
1725+
*/
1726+
public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled);
1727+
1728+
/**
1729+
* Determines if requests for a web application context root will be
1730+
* redirected (adding a trailing slash) by the Mapper. This is more
1731+
* efficient but has the side effect of confirming that the context path is
1732+
* valid.
1733+
*
1734+
* @return {@code true} if the Mapper level redirect is enabled for this
1735+
* Context.
1736+
*/
1737+
public boolean getMapperContextRootRedirectEnabled();
1738+
1739+
/**
1740+
* If enabled, requests for a directory will be redirected (adding a
1741+
* trailing slash) by the Mapper. This is more efficient but has the
1742+
* side effect of confirming that the directory is valid.
1743+
*
1744+
* @param mapperDirectoryRedirectEnabled Should the redirects be enabled?
1745+
*/
1746+
public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled);
1747+
1748+
/**
1749+
* Determines if requests for a directory will be redirected (adding a
1750+
* trailing slash) by the Mapper. This is more efficient but has the
1751+
* side effect of confirming that the directory is valid.
1752+
*
1753+
* @return {@code true} if the Mapper level redirect is enabled for this
1754+
* Context.
1755+
*/
1756+
public boolean getMapperDirectoryRedirectEnabled();
17171757
}

‎java/org/apache/catalina/core/StandardContext.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,13 +816,53 @@ public void unbind() {}
816816

817817
private boolean validateClientProvidedNewSessionId = true;
818818

819+
boolean mapperContextRootRedirectEnabled = false;
820+
821+
boolean mapperDirectoryRedirectEnabled = false;
822+
823+
819824
// ----------------------------------------------------- Context Properties
820825

826+
@Override
827+
public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) {
828+
this.mapperContextRootRedirectEnabled = mapperContextRootRedirectEnabled;
829+
}
830+
831+
832+
/**
833+
* {@inheritDoc}
834+
* <p>
835+
* The default value for this implementation is {@code false}.
836+
*/
837+
@Override
838+
public boolean getMapperContextRootRedirectEnabled() {
839+
return mapperContextRootRedirectEnabled;
840+
}
841+
842+
843+
@Override
844+
public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) {
845+
this.mapperDirectoryRedirectEnabled = mapperDirectoryRedirectEnabled;
846+
}
847+
848+
849+
/**
850+
* {@inheritDoc}
851+
* <p>
852+
* The default value for this implementation is {@code false}.
853+
*/
854+
@Override
855+
public boolean getMapperDirectoryRedirectEnabled() {
856+
return mapperDirectoryRedirectEnabled;
857+
}
858+
859+
821860
@Override
822861
public void setValidateClientProvidedNewSessionId(boolean validateClientProvidedNewSessionId) {
823862
this.validateClientProvidedNewSessionId = validateClientProvidedNewSessionId;
824863
}
825864

865+
826866
/**
827867
* {@inheritDoc}
828868
* <p>
@@ -833,6 +873,7 @@ public boolean getValidateClientProvidedNewSessionId() {
833873
return validateClientProvidedNewSessionId;
834874
}
835875

876+
836877
@Override
837878
public void setCookieProcessor(CookieProcessor cookieProcessor) {
838879
if (cookieProcessor == null) {

‎java/org/apache/catalina/core/mbeans-descriptors.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,14 @@
177177
description="Associated manager."
178178
type="org.apache.catalina.Manager" />
179179

180+
<attribute name="mapperContextRootRedirectEnabled"
181+
description="Should the Mapper be used for context root redirects"
182+
type="boolean" />
183+
184+
<attribute name="mapperDirectoryRedirectEnabled"
185+
description="Should the Mapper be used for directory redirects"
186+
type="boolean" />
187+
180188
<attribute name="namingContextListener"
181189
description="Associated naming context listener."
182190
type="org.apache.catalina.core.NamingContextListener" />

‎java/org/apache/catalina/mapper/Mapper.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,8 @@ private final void internalMapWrapper(ContextVersion contextVersion,
883883
}
884884
}
885885

886-
if(mappingData.wrapper == null && noServletPath) {
886+
if(mappingData.wrapper == null && noServletPath &&
887+
mappingData.context.getMapperContextRootRedirectEnabled()) {
887888
// The path is empty, redirect to "/"
888889
mappingData.redirectPath.setChars
889890
(path.getBuffer(), pathOffset, pathEnd-pathOffset);
@@ -1001,9 +1002,9 @@ private final void internalMapWrapper(ContextVersion contextVersion,
10011002
char[] buf = path.getBuffer();
10021003
if (contextVersion.resources != null && buf[pathEnd -1 ] != '/') {
10031004
String pathStr = path.toString();
1004-
WebResource file =
1005-
contextVersion.resources.getResource(pathStr);
1006-
if (file != null && file.isDirectory()) {
1005+
WebResource file = contextVersion.resources.getResource(pathStr);
1006+
if (file != null && file.isDirectory() &&
1007+
mappingData.context.getMapperDirectoryRedirectEnabled()) {
10071008
// Note: this mutates the path: do not do any processing
10081009
// after this (since we set the redirectPath, there
10091010
// shouldn't be any)
@@ -1020,7 +1021,6 @@ private final void internalMapWrapper(ContextVersion contextVersion,
10201021

10211022
path.setOffset(pathOffset);
10221023
path.setEnd(pathEnd);
1023-
10241024
}
10251025

10261026

‎java/org/apache/catalina/servlets/DefaultServlet.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,17 @@ protected void serveResource(HttpServletRequest request,
816816
long contentLength = -1L;
817817

818818
if (resource.isDirectory()) {
819+
if (!path.endsWith("/")) {
820+
StringBuilder location = new StringBuilder(request.getRequestURI());
821+
location.append('/');
822+
if (request.getQueryString() != null) {
823+
location.append('?');
824+
location.append(request.getQueryString());
825+
}
826+
response.sendRedirect(response.encodeRedirectURL(location.toString()));
827+
return;
828+
}
829+
819830
// Skip directory listings if we have been configured to
820831
// suppress them
821832
if (!listings) {

‎java/org/apache/catalina/startup/FailedContext.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,9 +764,25 @@ public void setCookieProcessor(CookieProcessor cookieProcessor) { /* NO-OP */ }
764764

765765
@Override
766766
public void setValidateClientProvidedNewSessionId(boolean validateClientProvidedNewSessionId) {
767-
//NO-OP
767+
// NO-OP
768768
}
769769

770770
@Override
771771
public boolean getValidateClientProvidedNewSessionId() { return false; }
772+
773+
@Override
774+
public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) {
775+
// NO-OP
776+
}
777+
778+
@Override
779+
public boolean getMapperContextRootRedirectEnabled() { return false; }
780+
781+
@Override
782+
public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) {
783+
// NO-OP
784+
}
785+
786+
@Override
787+
public boolean getMapperDirectoryRedirectEnabled() { return false; }
772788
}

‎test/org/apache/catalina/core/TesterContext.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,4 +1234,20 @@ public void setValidateClientProvidedNewSessionId(boolean validateClientProvided
12341234

12351235
@Override
12361236
public boolean getValidateClientProvidedNewSessionId() { return false; }
1237+
1238+
@Override
1239+
public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) {
1240+
// NO-OP
1241+
}
1242+
1243+
@Override
1244+
public boolean getMapperContextRootRedirectEnabled() { return false; }
1245+
1246+
@Override
1247+
public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) {
1248+
// NO-OP
1249+
}
1250+
1251+
@Override
1252+
public boolean getMapperDirectoryRedirectEnabled() { return false; }
12371253
}

‎test/org/apache/catalina/mapper/TestMapperWebapps.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.io.File;
2020
import java.io.IOException;
21+
import java.net.HttpURLConnection;
2122
import java.util.HashMap;
2223
import java.util.List;
2324

@@ -33,7 +34,10 @@
3334
import org.apache.catalina.core.StandardContext;
3435
import org.apache.catalina.startup.Tomcat;
3536
import org.apache.catalina.startup.TomcatBaseTest;
37+
import org.apache.catalina.valves.RemoteAddrValve;
3638
import org.apache.tomcat.util.buf.ByteChunk;
39+
import org.apache.tomcat.util.descriptor.web.SecurityCollection;
40+
import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
3741
import org.apache.tomcat.websocket.server.WsContextListener;
3842

3943
/**
@@ -225,6 +229,66 @@ public void testWelcomeFileStrict() throws Exception {
225229
Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc);
226230
}
227231

232+
@Test
233+
public void testRedirect() throws Exception {
234+
// Disable the following of redirects for this test only
235+
boolean originalValue = HttpURLConnection.getFollowRedirects();
236+
HttpURLConnection.setFollowRedirects(false);
237+
try {
238+
Tomcat tomcat = getTomcatInstance();
239+
240+
// Use standard test webapp as ROOT
241+
File rootDir = new File("test/webapp");
242+
org.apache.catalina.Context root =
243+
tomcat.addWebapp(null, "", rootDir.getAbsolutePath());
244+
245+
// Add a security constraint
246+
SecurityConstraint constraint = new SecurityConstraint();
247+
SecurityCollection collection = new SecurityCollection();
248+
collection.addPattern("/welcome-files/*");
249+
collection.addPattern("/welcome-files");
250+
constraint.addCollection(collection);
251+
constraint.addAuthRole("foo");
252+
root.addConstraint(constraint);
253+
254+
// Also make examples available
255+
File examplesDir = new File(getBuildDirectory(), "webapps/examples");
256+
org.apache.catalina.Context examples = tomcat.addWebapp(
257+
null, "/examples", examplesDir.getAbsolutePath());
258+
// Then block access to the examples to test redirection
259+
RemoteAddrValve rav = new RemoteAddrValve();
260+
rav.setDeny(".*");
261+
rav.setDenyStatus(404);
262+
examples.getPipeline().addValve(rav);
263+
264+
tomcat.start();
265+
266+
// Redirects within a web application
267+
doRedirectTest("/welcome-files", 401);
268+
doRedirectTest("/welcome-files/", 401);
269+
270+
doRedirectTest("/jsp", 302);
271+
doRedirectTest("/jsp/", 404);
272+
273+
doRedirectTest("/WEB-INF", 404);
274+
doRedirectTest("/WEB-INF/", 404);
275+
276+
// Redirects between web applications
277+
doRedirectTest("/examples", 404);
278+
doRedirectTest("/examples/", 404);
279+
} finally {
280+
HttpURLConnection.setFollowRedirects(originalValue);
281+
}
282+
}
283+
284+
285+
private void doRedirectTest(String path, int expected) throws IOException {
286+
ByteChunk bc = new ByteChunk();
287+
int rc = getUrl("http://localhost:" + getPort() + path, bc, null);
288+
Assert.assertEquals(expected, rc);
289+
}
290+
291+
228292
/**
229293
* Prepare a string to search in messages that contain a timestamp, when it
230294
* is known that the timestamp was printed between {@code timeA} and

‎test/org/apache/catalina/startup/TomcatBaseTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,8 +647,7 @@ public static int methodUrl(String path, ByteChunk out, int readTimeout,
647647
String method) throws IOException {
648648

649649
URL url = new URL(path);
650-
HttpURLConnection connection =
651-
(HttpURLConnection) url.openConnection();
650+
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
652651
connection.setUseCaches(false);
653652
connection.setReadTimeout(readTimeout);
654653
connection.setRequestMethod(method);

‎test/org/apache/catalina/valves/rewrite/TestRewriteValve.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.junit.Test;
2121

2222
import org.apache.catalina.Context;
23+
import org.apache.catalina.servlets.DefaultServlet;
2324
import org.apache.catalina.startup.Tomcat;
2425
import org.apache.catalina.startup.TomcatBaseTest;
2526
import org.apache.tomcat.util.buf.ByteChunk;
@@ -75,6 +76,8 @@ private void doTestRewrite(String config, String request, String expectedURI) th
7576
Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
7677
ctx.addServletMapping("/a/%255A", "snoop");
7778
ctx.addServletMapping("/c/*", "snoop");
79+
Tomcat.addServlet(ctx, "default", new DefaultServlet());
80+
ctx.addServletMapping("/", "default");
7881

7982
tomcat.start();
8083

0 commit comments

Comments
 (0)