diff --git a/packages/lib/getIP.test.ts b/packages/lib/getIP.test.ts index b5c3c3c523..fada4fae1d 100644 --- a/packages/lib/getIP.test.ts +++ b/packages/lib/getIP.test.ts @@ -1,6 +1,6 @@ import type { NextApiRequest } from "next"; -import { describe, expect, it } from "vitest"; -import getIP, { parseIpFromHeaders } from "./getIP"; +import { afterEach, describe, expect, it } from "vitest"; +import getIP, { isIpInBanlist, parseIpFromHeaders } from "./getIP"; function buildRequest(headers: Record): Request { return new Request("https://example.com", { headers }); @@ -22,6 +22,24 @@ describe("parseIpFromHeaders", () => { it("returns the first element when given an array", () => { expect(parseIpFromHeaders(["1.2.3.4", "5.6.7.8"])).toBe("1.2.3.4"); }); + + it("trims leading/trailing whitespace from a padded string value", () => { + expect(parseIpFromHeaders(" 1.2.3.4 , 5.6.7.8")).toBe("1.2.3.4"); + }); + + it("trims whitespace after splitting on comma", () => { + expect(parseIpFromHeaders("1.2.3.4 , 5.6.7.8")).toBe("1.2.3.4"); + }); + + it("trims leading/trailing whitespace from a padded array element", () => { + expect(parseIpFromHeaders([" 1.2.3.4 ", "5.6.7.8"])).toBe("1.2.3.4"); + }); + it("returns an empty string for an empty array instead of throwing", () => { + expect(parseIpFromHeaders([])).toBe(""); + }); + it("trims tabs and other whitespace, not just spaces", () => { + expect(parseIpFromHeaders("\t1.2.3.4\t, 5.6.7.8")).toBe("1.2.3.4"); + }); }); describe("getIP", () => { @@ -74,6 +92,24 @@ describe("getIP", () => { }); }); + describe("banlist bypass regression (#29851)", () => { + const originalBanlist = process.env.IP_BANLIST; + + afterEach(() => { + if (originalBanlist === undefined) { + delete process.env.IP_BANLIST; + } else { + process.env.IP_BANLIST = originalBanlist; + } + }); + + it("still detects a banned IP when x-forwarded-for is padded with whitespace", () => { + process.env.IP_BANLIST = JSON.stringify(["1.2.3.4"]); + const req = buildRequest({ "x-forwarded-for": " 1.2.3.4 , 5.6.7.8" }); + expect(isIpInBanlist(req)).toBe(true); + }); + }); + describe("with NextApiRequest", () => { it("returns cf-connecting-ip when present", () => { const req = buildNextApiRequest({ "cf-connecting-ip": "1.1.1.1" }); @@ -121,4 +157,4 @@ describe("getIP", () => { expect(getIP(req)).toBe("5.5.5.5"); }); }); -}); +}); \ No newline at end of file diff --git a/packages/lib/getIP.ts b/packages/lib/getIP.ts index 1962b417ce..94bef6b5a7 100644 --- a/packages/lib/getIP.ts +++ b/packages/lib/getIP.ts @@ -4,9 +4,11 @@ import z from "zod"; import logger from "./logger"; export function parseIpFromHeaders(value: string | string[]) { - return Array.isArray(value) ? value[0] : value.split(",")[0]; + const rawIp = Array.isArray(value) ? value[0] : value.split(",")[0]; + return rawIp?.trim() ?? ""; } + /** * Tries to extract IP address from a request. * @@ -53,4 +55,4 @@ export function isIpInBanListString(identifer: string) { return true; } return false; -} +} \ No newline at end of file