From cccd809b7d1c50da4f714574ff3bdbb6e7498e14 Mon Sep 17 00:00:00 2001 From: dmitrii Date: Wed, 24 Jun 2026 15:57:23 +0200 Subject: [PATCH] Strip port from the raw remote address, not just X-Forwarded-For ProxyForwardedParser only stripped an ephemeral source port when the IP came from X-Forwarded-For. When there is no trusted forwarding header it returns the raw socket address as-is, so an "ip:port" value (e.g. "1.2.3.4:58780") leaks through to attacks, rate limiting and the reported user IPs. On the cloud side this produces duplicate runtime-user IP rows (one per source port). Extract a stripPort() helper and apply it to the raw IP fallback as well. It handles IPv4 with a port and bracketed IPv6 with a port, and leaves already-valid IPs (including bare IPv6) and non-IP values untouched. The X-Forwarded-For parsing now reuses the same helper, which also adds bracketed IPv6 port stripping there. --- .../helpers/net/ProxyForwardedParser.java | 64 +++++++++++++++---- .../helpers/ProxyForwardedParserTest.java | 45 +++++++++++++ 2 files changed, 95 insertions(+), 14 deletions(-) diff --git a/agent_api/src/main/java/dev/aikido/agent_api/helpers/net/ProxyForwardedParser.java b/agent_api/src/main/java/dev/aikido/agent_api/helpers/net/ProxyForwardedParser.java index 3ec7b80f..1b2b9691 100644 --- a/agent_api/src/main/java/dev/aikido/agent_api/helpers/net/ProxyForwardedParser.java +++ b/agent_api/src/main/java/dev/aikido/agent_api/helpers/net/ProxyForwardedParser.java @@ -18,8 +18,11 @@ public static String getIpFromRequest(String rawIp, HashMap } } - // If no valid IP was found, or if X-Forwarded-For was not present, default to raw ip: - return rawIp; + // No (trusted) forwarding header: fall back to the raw socket address. + // Some servers/proxies hand us "ip:port" here too, so strip the port as + // well, otherwise the port ends up in attacks, rate limiting and the + // reported user IPs (which causes duplicate IPs on the cloud side). + return stripPort(rawIp); } /** @@ -30,22 +33,55 @@ public static boolean trustProxy() { return trustProxy.getValue(); } - private static String extractIpFromHeader(String xForwardedForHeader) { - String[] ips = xForwardedForHeader.split(","); - for (String ip: ips) { - ip = ip.trim(); + /** + * Strips a trailing port from an IP address, returning the bare IP. Handles + * IPv4 with a port ("1.2.3.4:5678") and bracketed IPv6 with a port + * ("[2001:db8::1]:5678"). Already-valid IPs (including bare IPv6) and values + * from which no port can be safely removed are returned unchanged. + */ + public static String stripPort(String ipAddress) { + if (ipAddress == null) { + return null; + } + String ip = ipAddress.trim(); + + if (IPValidator.isIP(ip)) { + return ip; + } - // Some proxies pass along port numbers inside x-forwarded-for : - if (ip.contains(":")) { - String[] ipParts = ip.split(":"); - if (ipParts.length == 2 && IPValidator.isIP(ipParts[0])) { - return ipParts[0]; + // Bracketed IPv6 with optional port, e.g. "[2001:db8::1]:5678". + if (ip.startsWith("[")) { + int closing = ip.indexOf(']'); + if (closing > 1) { + String inner = ip.substring(1, closing); + if (IPValidator.isIP(inner, "6")) { + return inner; } } + return ip; + } - // Continue to check the IPs : - if (IPValidator.isIP(ip)) { - return ip; + // IPv4 with a port, e.g. "1.2.3.4:5678". A single colon distinguishes this + // from a bare IPv6 address, which always has multiple colons. + int firstColon = ip.indexOf(':'); + if (firstColon > -1 && ip.indexOf(':', firstColon + 1) == -1) { + String candidate = ip.substring(0, firstColon); + if (IPValidator.isIP(candidate, "4")) { + return candidate; + } + } + + return ip; + } + + private static String extractIpFromHeader(String xForwardedForHeader) { + String[] ips = xForwardedForHeader.split(","); + for (String ip: ips) { + // Some proxies pass along port numbers inside x-forwarded-for, so + // strip them before validating. + String cleaned = stripPort(ip.trim()); + if (IPValidator.isIP(cleaned)) { + return cleaned; } } return null; diff --git a/agent_api/src/test/java/helpers/ProxyForwardedParserTest.java b/agent_api/src/test/java/helpers/ProxyForwardedParserTest.java index 22ee0b88..5a827579 100644 --- a/agent_api/src/test/java/helpers/ProxyForwardedParserTest.java +++ b/agent_api/src/test/java/helpers/ProxyForwardedParserTest.java @@ -7,6 +7,8 @@ import java.util.HashMap; import java.util.List; +import dev.aikido.agent_api.helpers.net.ProxyForwardedParser; + import static dev.aikido.agent_api.helpers.net.ProxyForwardedParser.getIpFromRequest; import static org.junit.jupiter.api.Assertions.*; @@ -145,4 +147,47 @@ void testGetIpFromRequest_MultipleValidIPsInXForwardedFor() { String result = getIpFromRequest("1.2.3.4", headers); assertEquals("5.6.7.8", result); } + + @Test + void testGetIpFromRequest_StripsPortFromRawIp() { + // No forwarding header present: the raw socket address can still carry a + // port, which must be stripped so it doesn't leak to the cloud side. + String result = getIpFromRequest("109.132.232.101:58780", new HashMap<>()); + assertEquals("109.132.232.101", result); + } + + @Test + @SetEnvironmentVariable(key = "AIKIDO_TRUST_PROXY", value = "0") + void testGetIpFromRequest_StripsPortFromRawIp_TrustProxyFalse() { + headers.put("X-Forwarded-For", List.of("192.168.1.1, 203.0.113.5")); + String result = getIpFromRequest("109.132.232.101:58780", headers); + assertEquals("109.132.232.101", result); + } + + @Test + void testGetIpFromRequest_KeepsBareRawIpv6() { + String result = getIpFromRequest("2001:db8::1", new HashMap<>()); + assertEquals("2001:db8::1", result); + } + + @Test + void testGetIpFromRequest_StripsPortFromBracketedIpv6InHeader() { + headers.put("X-Forwarded-For", List.of("[2001:db8::1]:8080, 203.0.113.5")); + String result = getIpFromRequest("10.0.0.1", headers); + assertEquals("2001:db8::1", result); + } + + @Test + void testStripPort() { + assertEquals("1.2.3.4", ProxyForwardedParser.stripPort("1.2.3.4")); + assertEquals("1.2.3.4", ProxyForwardedParser.stripPort("1.2.3.4:5678")); + assertEquals("1.2.3.4", ProxyForwardedParser.stripPort(" 1.2.3.4:5678 ")); + assertEquals("2001:db8::1", ProxyForwardedParser.stripPort("2001:db8::1")); + assertEquals("2001:db8::1", ProxyForwardedParser.stripPort("[2001:db8::1]:5678")); + assertEquals("2001:db8::1", ProxyForwardedParser.stripPort("[2001:db8::1]")); + // Nothing strippable: returned unchanged. + assertEquals("not-an-ip", ProxyForwardedParser.stripPort("not-an-ip")); + assertEquals("garbage:1234", ProxyForwardedParser.stripPort("garbage:1234")); + assertNull(ProxyForwardedParser.stripPort(null)); + } }