Skip to content

Commit f394d23

Browse files
hansotthansott
authored
Merge pull request #307 from AikidoSec/fix/runtime-user-ip-port-normalization
Normalize raw remote IP addresses with ports
2 parents 8987c88 + 893558b commit f394d23

4 files changed

Lines changed: 149 additions & 8 deletions

File tree

‎agent_api/src/main/java/dev/aikido/agent_api/helpers/net/ProxyForwardedParser.java‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ public static String getIpFromRequest(String rawIp, HashMap<String, List<String>
1919
}
2020

2121
// If no valid IP was found, or if X-Forwarded-For was not present, default to raw ip:
22+
String normalizedRawIp = normalizeIp(rawIp);
23+
if (normalizedRawIp != null) {
24+
return normalizedRawIp;
25+
}
26+
2227
return rawIp;
2328
}
2429

@@ -35,19 +40,50 @@ private static String extractIpFromHeader(String xForwardedForHeader) {
3540
for (String ip: ips) {
3641
ip = ip.trim();
3742

38-
// Some proxies pass along port numbers inside x-forwarded-for :
39-
if (ip.contains(":")) {
40-
String[] ipParts = ip.split(":");
41-
if (ipParts.length == 2 && IPValidator.isIP(ipParts[0])) {
42-
return ipParts[0];
43+
String normalizedIp = normalizeIp(ip);
44+
if (normalizedIp != null) {
45+
return normalizedIp;
46+
}
47+
}
48+
return null;
49+
}
50+
51+
private static String normalizeIp(String ip) {
52+
if (ip == null || ip.isEmpty()) {
53+
return null;
54+
}
55+
56+
ip = ip.trim();
57+
58+
if (IPValidator.isIP(ip)) {
59+
return ip;
60+
}
61+
62+
if (ip.startsWith("[") && ip.endsWith("]")) {
63+
String unwrappedIp = ip.substring(1, ip.length() - 1);
64+
if (IPValidator.isIP(unwrappedIp)) {
65+
return unwrappedIp;
66+
}
67+
}
68+
69+
if (ip.startsWith("[")) {
70+
int closingBracket = ip.indexOf("]:");
71+
if (closingBracket > 0) {
72+
String unwrappedIp = ip.substring(1, closingBracket);
73+
if (IPValidator.isIP(unwrappedIp)) {
74+
return unwrappedIp;
4375
}
4476
}
77+
}
4578

46-
// Continue to check the IPs :
47-
if (IPValidator.isIP(ip)) {
48-
return ip;
79+
// Some proxies pass along port numbers with IP addresses :
80+
if (ip.contains(":")) {
81+
String[] ipParts = ip.split(":");
82+
if (ipParts.length == 2 && IPValidator.isIP(ipParts[0], "4")) {
83+
return ipParts[0];
4984
}
5085
}
86+
5187
return null;
5288
}
5389

‎agent_api/src/test/java/SetUserTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import dev.aikido.agent_api.SetUser;
22
import dev.aikido.agent_api.context.Context;
33
import dev.aikido.agent_api.context.ContextObject;
4+
import dev.aikido.agent_api.context.JavalinContextObject;
45
import dev.aikido.agent_api.storage.UsersStore;
56
import org.junit.jupiter.api.*;
67
import org.junitpioneer.jupiter.SetEnvironmentVariable;
@@ -134,4 +135,25 @@ public void testWithContextSetButNotExecutedMiddleware(StdOut out) throws SQLExc
134135
assertFalse(out.capturedString().contains("SetUser")); // Should not contain SetUser class
135136
assertEquals(1, UsersStore.getUsersAsList().size());
136137
}
138+
139+
@Test
140+
void testSetUserUsesNormalizedIpFromContext() {
141+
ContextObject contextObject = new JavalinContextObject(
142+
"GET",
143+
"http://example.com",
144+
"109.132.232.101:58780",
145+
new HashMap<>(),
146+
new HashMap<>(),
147+
new HashMap<>()
148+
);
149+
150+
Context.set(contextObject);
151+
152+
SetUser.setUser(new SetUser.UserObject("admin", "Admin"));
153+
154+
assertNotNull(Context.get().getUser());
155+
assertEquals("109.132.232.101", Context.get().getUser().lastIpAddress());
156+
157+
Context.reset();
158+
}
137159
}

‎agent_api/src/test/java/context/JavalinContextObjectTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,18 @@ void testMultipleCookiesExtraction() {
103103
assertEquals("user456", contextObject.getCookies().get("userId").get(0));
104104
assertEquals(2, contextObject.getCookies().size());
105105
}
106+
107+
@Test
108+
void testConstructorStripsPortFromRawIp() {
109+
JavalinContextObject contextObject = new JavalinContextObject(
110+
"GET",
111+
"http://example.com",
112+
"109.132.232.101:58780",
113+
new HashMap<>(),
114+
new HashMap<>(),
115+
new HashMap<>()
116+
);
117+
118+
assertEquals("109.132.232.101", contextObject.getRemoteAddress());
119+
}
106120
}

‎agent_api/src/test/java/helpers/ProxyForwardedParserTest.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,73 @@ void testGetIpFromRequest_MultipleValidIPsInXForwardedFor() {
145145
String result = getIpFromRequest("1.2.3.4", headers);
146146
assertEquals("5.6.7.8", result);
147147
}
148+
149+
@Test
150+
void testGetIpFromRequest_RawIpWithIPv4Port() {
151+
String result = getIpFromRequest("109.132.232.101:58780", new HashMap<>());
152+
assertEquals("109.132.232.101", result);
153+
}
154+
155+
@Test
156+
void testGetIpFromRequest_RawIpWithBracketedIPv6Port() {
157+
String result = getIpFromRequest("[2001:db8::1]:443", new HashMap<>());
158+
assertEquals("2001:db8::1", result);
159+
}
160+
161+
@Test
162+
void testGetIpFromRequest_RawIpWithBracketedIPv6WithoutPort() {
163+
String result = getIpFromRequest("[2001:db8::1]", new HashMap<>());
164+
assertEquals("2001:db8::1", result);
165+
}
166+
167+
@Test
168+
void testGetIpFromRequest_XForwardedForWithBracketedIPv6Port() {
169+
headers.put("X-Forwarded-For", List.of("[2001:db8::1]:443, 203.0.113.5"));
170+
String result = getIpFromRequest("10.0.0.1", headers);
171+
assertEquals("2001:db8::1", result);
172+
}
173+
174+
@Test
175+
@SetEnvironmentVariable(key = "AIKIDO_TRUST_PROXY", value = "0")
176+
void testGetIpFromRequest_TrustProxyFalseStillNormalizesRawIp() {
177+
headers.put("X-Forwarded-For", List.of("1.2.3.4"));
178+
String result = getIpFromRequest("109.132.232.101:58780", headers);
179+
assertEquals("109.132.232.101", result);
180+
}
181+
182+
@Test
183+
void testGetIpFromRequest_RawIpWithPlainIPv4() {
184+
String result = getIpFromRequest("109.132.232.101", new HashMap<>());
185+
assertEquals("109.132.232.101", result);
186+
}
187+
188+
@Test
189+
void testGetIpFromRequest_RawIpWithPlainIPv6() {
190+
String result = getIpFromRequest("2001:db8::1", new HashMap<>());
191+
assertEquals("2001:db8::1", result);
192+
}
193+
194+
@Test
195+
void testGetIpFromRequest_RawIpWithInvalidBracketedIpFallsBackToRawIp() {
196+
String result = getIpFromRequest("[not-an-ip]", new HashMap<>());
197+
assertEquals("[not-an-ip]", result);
198+
}
199+
200+
@Test
201+
void testGetIpFromRequest_RawIpWithInvalidBracketedIpAndPortFallsBackToRawIp() {
202+
String result = getIpFromRequest("[not-an-ip]:443", new HashMap<>());
203+
assertEquals("[not-an-ip]:443", result);
204+
}
205+
206+
@Test
207+
void testGetIpFromRequest_RawIpWithInvalidIPv4PortFallsBackToRawIp() {
208+
String result = getIpFromRequest("not-an-ip:58780", new HashMap<>());
209+
assertEquals("not-an-ip:58780", result);
210+
}
211+
212+
@Test
213+
void testGetIpFromRequest_RawIpWithEmptyStringFallsBackToRawIp() {
214+
String result = getIpFromRequest("", new HashMap<>());
215+
assertEquals("", result);
216+
}
148217
}

0 commit comments

Comments
 (0)