This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 156eacc58b Fix BZ 70208 - Make URL encoding more robust.
156eacc58b is described below
commit 156eacc58bc694569a9d544925bb8898ae4e2925
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Sep 9 09:59:49 2026 +0100
Fix BZ 70208 - Make URL encoding more robust.
Based on pull request #1065 by Chenjp
---
java/org/apache/catalina/util/RequestUtil.java | 25 ++++++++++---
.../apache/catalina/connector/TestResponse.java | 41 +++++++++++++++++++---
test/org/apache/tomcat/unittest/TesterRequest.java | 14 +++++++-
.../tomcat/unittest/TesterServletContext.java | 41 +++++++++++-----------
webapps/docs/changelog.xml | 4 +++
5 files changed, 96 insertions(+), 29 deletions(-)
diff --git a/java/org/apache/catalina/util/RequestUtil.java
b/java/org/apache/catalina/util/RequestUtil.java
index b9a003620e..58f7ab9b29 100644
--- a/java/org/apache/catalina/util/RequestUtil.java
+++ b/java/org/apache/catalina/util/RequestUtil.java
@@ -17,11 +17,13 @@
package org.apache.catalina.util;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
import java.util.Enumeration;
import javax.servlet.http.HttpServletRequest;
import org.apache.catalina.connector.Request;
+import org.apache.tomcat.util.buf.UDecoder;
/**
* General purpose request parsing and encoding utility methods.
@@ -148,14 +150,29 @@ public final class RequestUtil {
}
/*
- * This isn't perfect but is the best that can be done without running
the full mapping logic on the url to
- * determine which web application that url will map to.
+ * May not be perfect, but make a best efforts attempt to determine
whether the URL belongs to current request
+ * or not.
+ *
+ * Stripping of path parameters, decoding and normalization should all
be unnecessary but are included here as
+ * hardening against the application using untrusted data when
constructing the URL.
*/
- if
(!url.getPath().startsWith(request.getServletContext().getContextPath())) {
+ String urlPath = url.getPath();
+ urlPath = stripPathParams(urlPath, null);
+ urlPath = UDecoder.URLDecode(urlPath, StandardCharsets.UTF_8);
+ urlPath = org.apache.tomcat.util.http.RequestUtil.normalize(urlPath);
+ if (urlPath == null) {
+ // Normalization failed. Path tried to escape the root.
return false;
}
- return true;
+ // Context path will not end with "/"
+ String requestContextPath =
request.getServletContext().getContextPath();
+
+ if (urlPath.equals(requestContextPath) ||
urlPath.startsWith(requestContextPath + "/")) {
+ return true;
+ }
+
+ return false;
}
diff --git a/test/org/apache/catalina/connector/TestResponse.java
b/test/org/apache/catalina/connector/TestResponse.java
index 98649fe66d..1a81836d93 100644
--- a/test/org/apache/catalina/connector/TestResponse.java
+++ b/test/org/apache/catalina/connector/TestResponse.java
@@ -349,7 +349,12 @@ public class TestResponse extends TomcatBaseTest {
private void doTestEncodeURL(String location, String expected) {
- Request req = new TesterRequest(true);
+ doTestEncodeURL("", location, expected);
+ }
+
+
+ private void doTestEncodeURL(String currentContextPath, String location,
String expected) {
+ Request req = new TesterRequest(true, "/level1/level2/foo.html",
currentContextPath);
req.setRequestedSessionId("1234");
req.setRequestedSessionURL(true);
Response resp = new Response();
@@ -455,6 +460,31 @@ public class TestResponse extends TomcatBaseTest {
}
+ @Test
+ public void testEncodeURLBug70208a() throws Exception {
+ doTestEncodeURL("/admin", "/admin/index",
"/admin/index;jsessionid=1234");
+ }
+
+
+ @Test
+ public void testEncodeURLBug70208b() throws Exception {
+ doTestEncodeURL("/admin", "/admin/../public/index",
"/admin/../public/index");
+ }
+
+
+ @Test
+ public void testEncodeURLBug70208c() throws Exception {
+ doTestEncodeURL("/admin", "/public/..;/admin/index;jsessionid=zzz",
+ "/public/..;/admin/index;jsessionid=zzz;jsessionid=1234");
+ }
+
+
+ @Test
+ public void testEncodeURLBug70208d() throws Exception {
+ doTestEncodeURL("/admin", "/administrator/index",
"/administrator/index");
+ }
+
+
private void doTestEncodeRedirectURL(String location, String expected) {
Request req = new TesterRequest(true);
req.setRequestedSessionId("1234");
@@ -1046,21 +1076,24 @@ public class TestResponse extends TomcatBaseTest {
response.setHeader("Content-Length", "10");
Assert.assertEquals(10, response.getContentLength());
Assert.assertEquals("10", response.getHeader("Content-Length"));
- Assert.assertEquals(1, response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
+ Assert.assertEquals(1,
+ response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
Assert.assertEquals(1, response.getHeaders("Content-Length").size());
// Invalid
response.setHeader("Content-Length", "zzz");
Assert.assertEquals(-1, response.getContentLength());
Assert.assertNull(response.getHeader("Content-Length"));
- Assert.assertEquals(0, response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
+ Assert.assertEquals(0,
+ response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
Assert.assertEquals(0, response.getHeaders("Content-Length").size());
// Valid
response.setHeader("Content-Length", "20");
Assert.assertEquals(20, response.getContentLength());
Assert.assertEquals("20", response.getHeader("Content-Length"));
- Assert.assertEquals(1, response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
+ Assert.assertEquals(1,
+ response.getHeaderNames().stream().filter(s ->
s.equalsIgnoreCase("Content-Length")).count());
Assert.assertEquals(1, response.getHeaders("Content-Length").size());
}
}
diff --git a/test/org/apache/tomcat/unittest/TesterRequest.java
b/test/org/apache/tomcat/unittest/TesterRequest.java
index c21882c102..93cae8d8a1 100644
--- a/test/org/apache/tomcat/unittest/TesterRequest.java
+++ b/test/org/apache/tomcat/unittest/TesterRequest.java
@@ -56,9 +56,14 @@ public class TesterRequest extends Request {
public TesterRequest(boolean withSession, String requestUri) {
+ this(withSession, requestUri, "");
+ }
+
+
+ public TesterRequest(boolean withSession, String requestUri, String
reqContextPath) {
super(null);
context = new TesterContext();
- servletContext = new TesterServletContext();
+ servletContext = new TesterServletContext(reqContextPath);
context.setServletContext(servletContext);
if (withSession) {
Set<SessionTrackingMode> modes = new HashSet<>();
@@ -78,11 +83,13 @@ public class TesterRequest extends Request {
return "http";
}
+
@Override
public String getServerName() {
return "localhost";
}
+
@Override
public int getServerPort() {
return 8080;
@@ -123,18 +130,22 @@ public class TesterRequest extends Request {
private String method;
+
public void setMethod(String method) {
this.method = method;
}
+
@Override
public String getMethod() {
return method;
}
private final Map<String,List<String>> headers = new HashMap<>();
+
public void addHeader(String name, String value) {
headers.computeIfAbsent(name, k -> new ArrayList<>()).add(value);
}
+
@Override
public String getHeader(String name) {
List<String> values = headers.get(name);
@@ -143,6 +154,7 @@ public class TesterRequest extends Request {
}
return values.get(0);
}
+
@Override
public Enumeration<String> getHeaders(String name) {
List<String> values = headers.get(name);
diff --git a/test/org/apache/tomcat/unittest/TesterServletContext.java
b/test/org/apache/tomcat/unittest/TesterServletContext.java
index e2d72ca567..4a4d236b30 100644
--- a/test/org/apache/tomcat/unittest/TesterServletContext.java
+++ b/test/org/apache/tomcat/unittest/TesterServletContext.java
@@ -43,14 +43,19 @@ import org.apache.tomcat.util.descriptor.web.FilterDef;
public class TesterServletContext implements ServletContext {
- /**
- * {@inheritDoc}
- * <p>
- * This test implementation is hard coded to return an empty String.
- */
+ private String contextPath = "";
+
+ public TesterServletContext() {
+ this("");
+ }
+
+ public TesterServletContext(String contextPath) {
+ this.contextPath = contextPath;
+ }
+
@Override
public String getContextPath() {
- return "";
+ return contextPath;
}
/**
@@ -66,8 +71,7 @@ public class TesterServletContext implements ServletContext {
/**
* {@inheritDoc}
* <p>
- * This test implementation is hard coded to return the class loader that
- * loaded this class.
+ * This test implementation is hard coded to return the class loader that
loaded this class.
*/
@Override
public ClassLoader getClassLoader() {
@@ -221,8 +225,7 @@ public class TesterServletContext implements ServletContext
{
}
@Override
- public Dynamic addServlet(String servletName,
- Class<? extends Servlet> servletClass) {
+ public Dynamic addServlet(String servletName, Class<? extends Servlet>
servletClass) {
throw new RuntimeException("Not implemented");
}
@@ -232,8 +235,7 @@ public class TesterServletContext implements ServletContext
{
}
@Override
- public <T extends Servlet> T createServlet(Class<T> c)
- throws ServletException {
+ public <T extends Servlet> T createServlet(Class<T> c) throws
ServletException {
throw new RuntimeException("Not implemented");
}
@@ -243,7 +245,7 @@ public class TesterServletContext implements ServletContext
{
}
@Override
- public Map<String, ? extends ServletRegistration>
getServletRegistrations() {
+ public Map<String,? extends ServletRegistration> getServletRegistrations()
{
throw new RuntimeException("Not implemented");
}
@@ -263,8 +265,7 @@ public class TesterServletContext implements ServletContext
{
}
@Override
- public <T extends Filter> T createFilter(Class<T> c)
- throws ServletException {
+ public <T extends Filter> T createFilter(Class<T> c) throws
ServletException {
throw new RuntimeException("Not implemented");
}
@@ -274,20 +275,21 @@ public class TesterServletContext implements
ServletContext {
}
@Override
- public Map<String, ? extends FilterRegistration> getFilterRegistrations() {
+ public Map<String,? extends FilterRegistration> getFilterRegistrations() {
throw new RuntimeException("Not implemented");
}
private SessionCookieConfig sessionCookieConfig = new
TesterSessionCookieConfig();
+
@Override
public SessionCookieConfig getSessionCookieConfig() {
return sessionCookieConfig;
}
private final Set<SessionTrackingMode> sessionTrackingModes = new
HashSet<>();
+
@Override
- public void setSessionTrackingModes(
- Set<SessionTrackingMode> sessionTrackingModes) {
+ public void setSessionTrackingModes(Set<SessionTrackingMode>
sessionTrackingModes) {
this.sessionTrackingModes.clear();
this.sessionTrackingModes.addAll(sessionTrackingModes);
}
@@ -318,8 +320,7 @@ public class TesterServletContext implements ServletContext
{
}
@Override
- public <T extends EventListener> T createListener(Class<T> c)
- throws ServletException {
+ public <T extends EventListener> T createListener(Class<T> c) throws
ServletException {
throw new RuntimeException("Not implemented");
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 641c970b8f..f863a6232b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -213,6 +213,10 @@
Require the request to the login action during FORM authentication to
be
made using HTTP POST. (markt)
</fix>
+ <fix>
+ <bug>70208</bug>: Make URL encoding more robust. Based on pull request
+ <pr>1065</pr> by Chenjp. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
