From d80c622b005c044e93f585c231b420a29371f6e0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 24 Mar 2021 12:54:06 +0800 Subject: [PATCH] Merge pull request from GHSA-v7ff-8wcx-gmc5 (#6089) Always normalize ambiguous URIs Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/server/Request.java | 11 ++++- .../jetty/webapp/WebAppContextTest.java | 40 +++++++++++++++++++ jetty-webapp/src/test/webapp/WEB-INF/test.xml | 1 + jetty-webapp/src/test/webapp/test.xml | 1 + 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 jetty-webapp/src/test/webapp/WEB-INF/test.xml create mode 100644 jetty-webapp/src/test/webapp/test.xml diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 1812eea6d4c..27bf9d2e3b7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1692,7 +1692,8 @@ public class Request implements HttpServletRequest _httpFields = request.getFields(); final HttpURI uri = request.getURI(); - if (uri.isAmbiguous()) + boolean ambiguous = uri.isAmbiguous(); + if (ambiguous) { UriCompliance compliance = _channel == null || _channel.getHttpConfiguration() == null ? null : _channel.getHttpConfiguration().getUriCompliance(); if (uri.hasAmbiguousSegment() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT))) @@ -1741,7 +1742,15 @@ public class Request implements HttpServletRequest // TODO this is not really right for CONNECT path = _uri.isAbsolute() ? "/" : null; else if (encoded.startsWith("/")) + { path = (encoded.length() == 1) ? "/" : _uri.getDecodedPath(); + // Strictly speaking if a URI is legal and encodes ambiguous segments, then they should be + // reflected in the decoded string version. However, it can be ambiguous to provide a decoded path as + // a string, so we normalize again. If an application wishes to see ambiguous URIs, then they can look + // at the encoded form of the URI + if (ambiguous) + path = URIUtil.canonicalPath(path); + } else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod())) path = encoded; else diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 8d8947cd887..88f2380cced 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -28,8 +28,11 @@ import javax.servlet.ServletContext; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.UriCompliance; +import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -284,6 +287,43 @@ public class WebAppContextTest assertFalse(context.isProtectedTarget("/something-else/web-inf")); } + @Test + public void testProtectedTarget() throws Exception + { + Server server = newServer(); + + HandlerList handlers = new HandlerList(); + ContextHandlerCollection contexts = new ContextHandlerCollection(); + WebAppContext context = new WebAppContext(); + Path testWebapp = MavenTestingUtils.getProjectDirPath("src/test/webapp"); + context.setBaseResource(new PathResource(testWebapp)); + context.setContextPath("/"); + server.setHandler(handlers); + handlers.addHandler(contexts); + contexts.addHandler(context); + + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + + server.start(); + + assertThat(HttpTester.parseResponse(connector.getResponse("GET /test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + + assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/ HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /web-inf/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2E/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET //WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF%2ftest.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + } + @Test public void testNullPath() throws Exception { diff --git a/jetty-webapp/src/test/webapp/WEB-INF/test.xml b/jetty-webapp/src/test/webapp/WEB-INF/test.xml new file mode 100644 index 00000000000..9daeafb9864 --- /dev/null +++ b/jetty-webapp/src/test/webapp/WEB-INF/test.xml @@ -0,0 +1 @@ +test diff --git a/jetty-webapp/src/test/webapp/test.xml b/jetty-webapp/src/test/webapp/test.xml new file mode 100644 index 00000000000..9daeafb9864 --- /dev/null +++ b/jetty-webapp/src/test/webapp/test.xml @@ -0,0 +1 @@ +test