diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java
index 17c937381af9..2aab561b8853 100644
--- a/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ b/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -726,8 +726,11 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F
// loop on remoteIpHeaderValue to find the first trusted remote ip and to build the proxies chain
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
+ if (!isValidRemoteIp(currentRemoteIp)) {
+ // Ignore invalid remote ip header value
+ continue;
+ }
remoteIp = currentRemoteIp;
-
if (isInternalProxy(currentRemoteIp)) {
// do nothing, internalProxies IPs are not appended to the
} else if (isTrustedProxy(currentRemoteIp)) {
@@ -880,6 +883,28 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) {
return false;
}
+ /**
+ * Check the remote ip value with a basic syntax formating.
+ * @param remoteIp the header value from request
+ * @return false if the remote ip is null, empty, or malformed. Otherwise returns true
+ */
+ private boolean isValidRemoteIp(String remoteIp) {
+ if (remoteIp == null || remoteIp.isBlank()) {
+ return false;
+ }
+ // acceptable regular chars: 0-9/A-Z/a-z
+ // acceptable special chars: []:.-
+ for (char c : remoteIp.toCharArray()) {
+ if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '[' || c == ']'
+ || c == ':' || c == '.' || c == '-') {
+ continue;
+ } else {
+ return false;
+ }
+ }
+ return true;
+ }
+
/*
* Considers the value to be secure if it exclusively holds forwards for {@link #protocolHeaderHttpsValue}.
*/
diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index a10b09c40766..166041edf89b 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -555,6 +555,10 @@ public void invoke(Request request, Response response) throws IOException, Servl
// loop on remoteIpHeaderValue to find the first trusted remote ip and to build the proxies chain
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
+ if(!isValidRemoteIp(currentRemoteIp)) {
+ // Ignore invalid remote ip header value
+ continue;
+ }
remoteIp = currentRemoteIp;
if (isInternalProxy(currentRemoteIp)) {
// do nothing, internalProxies IPs are not appended to the
@@ -737,6 +741,28 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) {
return false;
}
+ /**
+ * Check the remote ip value with a basic syntax formating.
+ * @param remoteIp the header value from request
+ * @return false if the remote ip is null, empty, or malformed. Otherwise returns true
+ */
+ private boolean isValidRemoteIp(String remoteIp) {
+ if (remoteIp == null || remoteIp.isBlank()) {
+ return false;
+ }
+ // acceptable regular chars: 0-9/A-Z/a-z
+ // acceptable special chars: []:.-
+ for (char c : remoteIp.toCharArray()) {
+ if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '[' || c == ']'
+ || c == ':' || c == '.' || c == '-') {
+ continue;
+ } else {
+ return false;
+ }
+ }
+ return true;
+ }
+
/*
* Considers the value to be secure if it exclusively holds forwards for {@link #protocolHeaderHttpsValue}.
*/
diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
index 5243a3c62359..2ebb17a1c767 100644
--- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -558,6 +558,40 @@ public void testInvokeUntrustedProxyInTheChain() throws Exception {
Assert.assertEquals("remoteHost", "untrusted-proxy", actualRemoteHost);
}
+ @Test
+ public void testInvokeInvalidProxyInTheChain() throws Exception {
+ // PREPARE
+ FilterDef filterDef = new FilterDef();
+ filterDef.addInitParameter("internalProxies", "192.168.0.10/31");
+ filterDef.addInitParameter("trustedProxies", "200.0.0.1,200.0.0.2,200.0.0.3");
+ filterDef.addInitParameter("remoteIpHeader", "x-forwarded-for");
+ filterDef.addInitParameter("proxiesHeader", "x-forwarded-by");
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+
+ request.setRemoteAddr("192.168.0.10");
+ request.setRemoteHost("remote-host-original-value");
+ request.setHeader("x-forwarded-for", "140.211.11.130, 200.0.0.1, untrusted-proxy, invalid-proxy 01, 200.0.0.2");
+
+ // TEST
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request).getRequest();
+
+ // VERIFY
+ String actualXForwardedFor = actualRequest.getHeader("x-forwarded-for");
+ Assert.assertEquals("syntax invalid proxy should ignore, ip/host before untrusted-proxy must appear in x-forwarded-for", "140.211.11.130,200.0.0.1",
+ actualXForwardedFor);
+
+ String actualXForwardedBy = actualRequest.getHeader("x-forwarded-by");
+ Assert.assertEquals("syntax invalid proxy should ignore, ip/host after untrusted-proxy must appear in x-forwarded-by", "200.0.0.2",
+ actualXForwardedBy);
+
+ String actualRemoteAddr = actualRequest.getRemoteAddr();
+ Assert.assertEquals("remoteAddr", "untrusted-proxy", actualRemoteAddr);
+
+ String actualRemoteHost = actualRequest.getRemoteHost();
+ Assert.assertEquals("remoteHost", "untrusted-proxy", actualRemoteHost);
+ }
+
@Test
public void testInvokeXforwardedHost() throws Exception {
// PREPARE
diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index d89a4ec60115..b58778cc0bb6 100644
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -1103,6 +1103,34 @@ public void testRequestForwarded() throws Exception {
request.getAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE));
}
+ @Test
+ public void testIgnoreInvalidRequestForwardedFor() throws Exception {
+
+ // PREPARE
+ RemoteIpValve remoteIpValve = new RemoteIpValve();
+ remoteIpValve.setRemoteIpHeader("x-forwarded-for");
+ remoteIpValve.setProtocolHeader("x-forwarded-proto");
+ RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve();
+ remoteIpValve.setNext(remoteAddrAndHostTrackerValve);
+
+ Request request = new MockRequest(new org.apache.coyote.Request());
+ // client ip
+ request.setRemoteAddr("192.168.0.10");
+ request.setRemoteHost("192.168.0.10");
+ request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130,\"proxy \"01");
+ // protocol
+ request.setServerPort(8080);
+ request.getCoyoteRequest().scheme().setString("http");
+
+ // TEST
+ remoteIpValve.invoke(request, null);
+
+ // VERIFY
+ Assert.assertEquals("org.apache.tomcat.request.forwarded", Boolean.TRUE,
+ request.getAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE));
+ Assert.assertEquals("Valid remote ip header expected", "140.211.11.130", request.getAttribute(AccessLog.REMOTE_ADDR_ATTRIBUTE));
+ }
+
private void assertArrayEquals(String[] expected, String[] actual) {
if (expected == null) {
Assert.assertNull(actual);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0ecbb5b9bc46..b7a302e7e9e7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -210,6 +210,11 @@
70000: fix duplication of special headers in the response
after commit, following fix for 69967. (remm)
+
+ Fix remote ip/host header population in remote ip valve or filter,
+ ignore malformed header value. Those unexpected malformed remote ip/host
+ header value may mislead downstream components. Patch submitted by Chenjp.
+