From f3aad91c8ace2bacce9732d2a37716ee3f027b1f Mon Sep 17 00:00:00 2001 From: chenjipeng Date: Thu, 16 Apr 2026 17:07:25 +0800 Subject: [PATCH 1/4] Ignore invalid remote ip header value Per de-fact standards Enforce remote ip chars checking, only following char is acceptable: 0123456789abcdeABCDE[]: Ignore invalid header value. --- .../catalina/filters/RemoteIpFilter.java | 18 +++++++++++++++++- .../apache/catalina/valves/RemoteIpValve.java | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java index 17c937381af9..2da206018365 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,19 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { return false; } + private boolean isValidRemoteIp(String value) { + // Valid chars of remote ip: 0123456789abcdeABCDE[]: + for (char c : value.toCharArray()) { + if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'e') || (c >= 'A' && c <= 'E') || 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..a893cf264408 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,18 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { return false; } + private boolean isValidRemoteIp(String value) { + // Valid chars of remote ip: 0123456789abcdeABCDE[]: + for (char c : value.toCharArray()) { + if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'e') || (c >= 'A' && c <= 'E') || c == '[' || c == ']' + || c == ':') { + } else { + return false; + } + } + return true; + } + /* * Considers the value to be secure if it exclusively holds forwards for {@link #protocolHeaderHttpsValue}. */ From bd2334ff294b9bffb5664993ac28659d8cfb510d Mon Sep 17 00:00:00 2001 From: chenjipeng Date: Fri, 17 Apr 2026 09:42:28 +0800 Subject: [PATCH 2/4] May put proxy name into remote ip heaer Modify validation check. add testcase. --- .../catalina/filters/RemoteIpFilter.java | 6 ++-- .../apache/catalina/valves/RemoteIpValve.java | 7 ++-- .../catalina/filters/TestRemoteIpFilter.java | 34 +++++++++++++++++++ .../catalina/valves/TestRemoteIpValve.java | 28 +++++++++++++++ 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java index 2da206018365..ba77c8da7f93 100644 --- a/java/org/apache/catalina/filters/RemoteIpFilter.java +++ b/java/org/apache/catalina/filters/RemoteIpFilter.java @@ -884,10 +884,10 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { } private boolean isValidRemoteIp(String value) { - // Valid chars of remote ip: 0123456789abcdeABCDE[]: + // Valid chars of remote ip: 0123456789abcdeABCDE[]:.-_ for (char c : value.toCharArray()) { - if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'e') || (c >= 'A' && c <= 'E') || c == '[' || c == ']' - || c == ':') { + if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '[' || c == ']' + || c == ':' || c == '.' || c == '-' || c == '_') { continue; } else { return false; diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java index a893cf264408..5a4b56ba78eb 100644 --- a/java/org/apache/catalina/valves/RemoteIpValve.java +++ b/java/org/apache/catalina/valves/RemoteIpValve.java @@ -742,10 +742,11 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { } private boolean isValidRemoteIp(String value) { - // Valid chars of remote ip: 0123456789abcdeABCDE[]: + // Valid chars of remote ip: 0123456789abcdeABCDE[]:.-_ for (char c : value.toCharArray()) { - if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'e') || (c >= 'A' && c <= 'E') || c == '[' || c == ']' - || c == ':') { + if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '[' || c == ']' + || c == ':' || c == '.' || c == '-' || c == '_') { + continue; } else { return false; } 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); From 7266ee2c9e3288d76602af6760269fd454ab8ffc Mon Sep 17 00:00:00 2001 From: chenjipeng Date: Fri, 17 Apr 2026 10:33:50 +0800 Subject: [PATCH 3/4] More work Consider the possibility of empty remote ip value. Format code. --- .../catalina/filters/RemoteIpFilter.java | 19 ++++++++++++++----- .../apache/catalina/valves/RemoteIpValve.java | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java index ba77c8da7f93..2aab561b8853 100644 --- a/java/org/apache/catalina/filters/RemoteIpFilter.java +++ b/java/org/apache/catalina/filters/RemoteIpFilter.java @@ -883,11 +883,20 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { return false; } - private boolean isValidRemoteIp(String value) { - // Valid chars of remote ip: 0123456789abcdeABCDE[]:.-_ - for (char c : value.toCharArray()) { - if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '[' || c == ']' - || c == ':' || c == '.' || c == '-' || c == '_') { + /** + * 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; diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java index 5a4b56ba78eb..166041edf89b 100644 --- a/java/org/apache/catalina/valves/RemoteIpValve.java +++ b/java/org/apache/catalina/valves/RemoteIpValve.java @@ -741,11 +741,20 @@ private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) { return false; } - private boolean isValidRemoteIp(String value) { - // Valid chars of remote ip: 0123456789abcdeABCDE[]:.-_ - for (char c : value.toCharArray()) { - if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '[' || c == ']' - || c == ':' || c == '.' || c == '-' || c == '_') { + /** + * 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; From 1d662ec8ec7a738167bb6062c15f936f30e87f44 Mon Sep 17 00:00:00 2001 From: chenjipeng Date: Fri, 17 Apr 2026 11:33:15 +0800 Subject: [PATCH 4/4] Update changelog.xml --- webapps/docs/changelog.xml | 5 +++++ 1 file changed, 5 insertions(+) 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. +