diff --git a/src/lib/__tests__/rateLimit.test.ts b/src/lib/__tests__/rateLimit.test.ts index 864a3123..441d33cb 100644 --- a/src/lib/__tests__/rateLimit.test.ts +++ b/src/lib/__tests__/rateLimit.test.ts @@ -97,13 +97,14 @@ describe("RateLimiter", () => { }); describe("getClientIp", () => { - it("returns the right-most untrusted x-forwarded-for IP", () => { + it("returns the right-most valid x-forwarded-for IP without skipping private IPs", () => { const req = new Request("http://localhost", { headers: { - "x-forwarded-for": "5.6.7.8, 9.10.11.12, 10.0.0.1" // 10.0.0.1 is trusted proxy + "x-forwarded-for": "5.6.7.8, 9.10.11.12, 10.0.0.1" } }); - expect(getClientIp(req)).toBe("9.10.11.12"); + // 10.0.0.1 should not be skipped anymore, it is the right-most valid IP. + expect(getClientIp(req)).toBe("10.0.0.1"); }); it("trims whitespace from the selected x-forwarded-for IP", () => { @@ -147,7 +148,7 @@ describe("getClientIp", () => { expect(getClientIp(req)).toBe("unknown"); }); - it("returns the first valid untrusted IP when the right-most x-forwarded-for token is empty", () => { + it("returns the first valid IP when the right-most x-forwarded-for token is empty", () => { const req = new Request("http://localhost", { headers: { "x-forwarded-for": "5.6.7.8, " @@ -156,7 +157,7 @@ describe("getClientIp", () => { expect(getClientIp(req)).toBe("5.6.7.8"); }); - it("returns the first valid untrusted IP when the right-most x-forwarded-for token is invalid", () => { + it("returns the first valid IP when the right-most x-forwarded-for token is invalid", () => { const req = new Request("http://localhost", { headers: { "x-forwarded-for": "5.6.7.8, not-an-ip" @@ -164,39 +165,13 @@ describe("getClientIp", () => { }); expect(getClientIp(req)).toBe("5.6.7.8"); }); - it.each([ - ["IPv4 loopback", "203.0.113.10, 127.0.0.2"], - ["IPv4 link-local", "203.0.113.10, 169.254.10.20"], - ["IPv4-mapped IPv6 private IPv4", "203.0.113.10, ::ffff:10.0.0.1"], - ["IPv4-mapped IPv6 loopback", "203.0.113.10, ::ffff:127.0.0.2"], - ["IPv4-mapped IPv6 link-local", "203.0.113.10, ::ffff:169.254.10.20"], - ["IPv6 ULA fc prefix", "203.0.113.10, fc12::1"], - ["IPv6 ULA uppercase fd prefix", "203.0.113.10, FD00::1"], - ["IPv6 link-local", "203.0.113.10, fe80::1"], - ])("skips trusted proxy range: %s", (_, forwardedFor) => { - const req = new Request("http://localhost", { - headers: { - "x-forwarded-for": forwardedFor - } - }); - expect(getClientIp(req)).toBe("203.0.113.10"); - }); - - it("returns unknown if all x-forwarded-for IPs are trusted proxies", () => { - const req = new Request("http://localhost", { - headers: { - "x-forwarded-for": "192.168.1.1, 10.0.0.1" - } - }); - expect(getClientIp(req)).toBe("unknown"); - }); - it("returns unknown if a spoofed private chain contains no public client IP", () => { + it("does not skip spoofed private chains", () => { const req = new Request("http://localhost", { headers: { "x-forwarded-for": "127.0.0.2, 169.254.10.20, fd00::1" } }); - expect(getClientIp(req)).toBe("unknown"); + expect(getClientIp(req)).toBe("fd00::1"); }); }); diff --git a/src/lib/rateLimit.ts b/src/lib/rateLimit.ts index 4ea5c6e7..8f06dbe9 100644 --- a/src/lib/rateLimit.ts +++ b/src/lib/rateLimit.ts @@ -66,31 +66,18 @@ function isValidIp(value: string): boolean { } } -function isTrustedProxy(ip: string): boolean { - // Matches private IPv4 ranges, loopback, and link-local addresses. - const privateIpv4 = /^(127\.|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/; - // Matches IPv4-mapped IPv6 versions of the same private IPv4 ranges. - const privateIpv4MappedIpv6 = /^::ffff:(127\.|10\.|192\.168\.|169\.254\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/i; - // Matches IPv6 loopback, ULA (fc00::/7), and link-local (fe80::/10). - const privateIpv6 = /^(::1|f[cd]|fe[89ab])/i; - - return ( - privateIpv4.test(ip) || - privateIpv4MappedIpv6.test(ip) || - privateIpv6.test(ip) - ); -} - export function getClientIp(request: Request): string { const forwardedFor = request.headers.get("x-forwarded-for"); if (!forwardedFor) return "unknown"; const ips = forwardedFor.split(",").map(ip => ip.trim()); - // Iterate from right to left and return the first non-private IP as the client IP. + // Iterate from right to left and return the first valid IP. + // We do not skip private IPs because doing so without a known proxy topology + // allows attackers to bypass rate limiting by appending a spoofed private IP. for (let i = ips.length - 1; i >= 0; i--) { const ip = ips[i]; - if (ip && isValidIp(ip) && !isTrustedProxy(ip)) { + if (ip && isValidIp(ip)) { return ip; } }