From d773bf7720c7456a32a6bfd15cf297ba7772d9fc Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 25 Jun 2026 21:26:11 -0300 Subject: [PATCH] fix(auth): only trust forwarding headers from loopback TCP peers (#4689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrated into release/v3.8.37 — loopback-gated forwarding headers (IP spoofing fix). Cherry-picked onto current release tip; ipUtils.test.ts 9/9 green. --- src/lib/ipUtils.ts | 54 +++++++++++++++++++++++----- tests/unit/ipUtils.test.ts | 74 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 tests/unit/ipUtils.test.ts diff --git a/src/lib/ipUtils.ts b/src/lib/ipUtils.ts index b1881b28d2..3bf92d3127 100644 --- a/src/lib/ipUtils.ts +++ b/src/lib/ipUtils.ts @@ -26,9 +26,41 @@ export function extractClientIp( return remoteAddress?.trim() ?? "unknown"; } +/** + * Strip an IPv4-mapped IPv6 prefix ("::ffff:127.0.0.1" -> "127.0.0.1") so the + * loopback check below catches both representations Node may report. + */ +function normalizePeer(addr: string | undefined): string { + const trimmed = (addr ?? "").trim(); + if (!trimmed) return ""; + return trimmed.startsWith("::ffff:") ? trimmed.slice("::ffff:".length) : trimmed; +} + +/** + * Whether the TCP peer is a loopback address (i.e. the request reached us via + * a local reverse proxy such as nginx). Only then is it safe to trust the + * forwarding headers — from a direct public socket those headers are + * attacker-controlled and must be ignored, otherwise per-IP brute-force + * buckets (login lockout, etc.) become spoofable / shareable. + * + * Ported from decolua/9router#1893. + */ +function isLoopbackPeer(addr: string | undefined): boolean { + const ip = normalizePeer(addr); + if (!ip) return false; + if (ip === "::1") return true; + return ip.startsWith("127."); +} + /** * Extract client IP from a Request or NextRequest object. - * Checks X-Forwarded-For, X-Real-IP, CF-Connecting-IP, then socket. + * + * Behind a local reverse proxy (TCP peer is loopback) we trust the standard + * forwarding headers in priority order: CF-Connecting-IP > X-Forwarded-For > + * X-Real-IP. Directly from a public socket those headers are spoofable, so we + * key by the unspoofable TCP peer address instead. When no peer is known + * (edge runtime / fetch path with no socket) we fall back to the headers so + * we don't regress to "unknown" for every request in that path. */ export function getClientIpFromRequest(req: { headers?: Headers | { get?: (n: string) => string | null }; @@ -44,13 +76,19 @@ export function getClientIpFromRequest(req: { return null; }; - // Priority: CF-Connecting-IP (Cloudflare) > X-Forwarded-For > X-Real-IP > socket - const cfIp = getHeader("cf-connecting-ip"); - if (cfIp && isIP(cfIp.trim()) !== 0) return cfIp.trim(); - - const xff = getHeader("x-forwarded-for"); - const realIp = getHeader("x-real-ip"); const remoteAddress = req.ip ?? req.socket?.remoteAddress; + const hasPeer = Boolean(normalizePeer(remoteAddress)); + const trustForwardingHeaders = !hasPeer || isLoopbackPeer(remoteAddress); - return extractClientIp(xff ?? realIp, remoteAddress); + if (trustForwardingHeaders) { + const cfIp = getHeader("cf-connecting-ip"); + if (cfIp && isIP(cfIp.trim()) !== 0) return cfIp.trim(); + + const xff = getHeader("x-forwarded-for"); + const realIp = getHeader("x-real-ip"); + return extractClientIp(xff ?? realIp, remoteAddress); + } + + // Direct public peer — forwarding headers are attacker-controlled, ignore. + return normalizePeer(remoteAddress); } diff --git a/tests/unit/ipUtils.test.ts b/tests/unit/ipUtils.test.ts new file mode 100644 index 0000000000..cb1c83f638 --- /dev/null +++ b/tests/unit/ipUtils.test.ts @@ -0,0 +1,74 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { extractClientIp, getClientIpFromRequest } from "@/lib/ipUtils"; + +/** + * Regression tests for IP detection — ported from decolua/9router#1893. + * + * When OmniRoute runs behind a local reverse proxy (nginx etc.) the TCP peer + * is loopback (127.0.0.1 / ::1) and forwarding headers (X-Forwarded-For, + * X-Real-IP, CF-Connecting-IP) carry the real client IP. When the request + * arrives directly from the public internet, the TCP peer IS the client and + * forwarding headers are spoofable — keying brute-force buckets by them lets + * one attacker either lock everyone else out or evade the lockout entirely. + */ + +function makeReq(headers: Record, remoteAddress?: string) { + return { + headers: new Headers(headers), + socket: remoteAddress ? { remoteAddress } : undefined, + }; +} + +describe("ipUtils — loopback-gated forwarding headers", () => { + it("trusts X-Forwarded-For when TCP peer is 127.0.0.1 loopback", () => { + const req = makeReq({ "x-forwarded-for": "203.0.113.10" }, "127.0.0.1"); + assert.equal(getClientIpFromRequest(req), "203.0.113.10"); + }); + + it("trusts X-Forwarded-For when TCP peer is ::1 loopback", () => { + const req = makeReq({ "x-forwarded-for": "203.0.113.11" }, "::1"); + assert.equal(getClientIpFromRequest(req), "203.0.113.11"); + }); + + it("trusts X-Real-IP when TCP peer is loopback", () => { + const req = makeReq({ "x-real-ip": "203.0.113.12" }, "127.0.0.1"); + assert.equal(getClientIpFromRequest(req), "203.0.113.12"); + }); + + it("trusts CF-Connecting-IP when TCP peer is loopback", () => { + const req = makeReq({ "cf-connecting-ip": "203.0.113.13" }, "127.0.0.1"); + assert.equal(getClientIpFromRequest(req), "203.0.113.13"); + }); + + it("ignores spoofed X-Forwarded-For when TCP peer is a public address", () => { + // Direct public client trying to spoof another IP — must be ignored so + // the brute-force guard keys by the unspoofable TCP peer. + const req = makeReq({ "x-forwarded-for": "203.0.113.99" }, "198.51.100.5"); + assert.equal(getClientIpFromRequest(req), "198.51.100.5"); + }); + + it("ignores spoofed CF-Connecting-IP when TCP peer is a public address", () => { + const req = makeReq({ "cf-connecting-ip": "203.0.113.88" }, "198.51.100.7"); + assert.equal(getClientIpFromRequest(req), "198.51.100.7"); + }); + + it("falls back to forwarding headers when no socket peer is known", () => { + // Edge runtime / fetch path where req.socket is absent — preserve prior + // behavior, otherwise we'd lose all IPs in that path. + const req = makeReq({ "x-forwarded-for": "203.0.113.20" }); + assert.equal(getClientIpFromRequest(req), "203.0.113.20"); + }); + + it("returns loopback peer when no forwarding headers are present", () => { + const req = makeReq({}, "127.0.0.1"); + assert.equal(getClientIpFromRequest(req), "127.0.0.1"); + }); + + it("extractClientIp (lower-level) keeps prior contract", () => { + // Lower-level helper does NOT know the peer — preserve prior behavior. + assert.equal(extractClientIp("203.0.113.1, 10.0.0.1", "127.0.0.1"), "203.0.113.1"); + assert.equal(extractClientIp(null, "198.51.100.1"), "198.51.100.1"); + assert.equal(extractClientIp("unknown, 203.0.113.2", "10.0.0.1"), "203.0.113.2"); + }); +});