fix: trim whitespace in parseIpFromHeaders to prevent IP banlist bypass (#29857)

* fix: trim whitespace in parseIpFromHeaders to prevent IP banlist bypass

Fixes #29851

parseIpFromHeaders did not trim whitespace after splitting on comma
(or from array elements), so a padded x-forwarded-for header like
' 1.2.3.4 , 5.6.7.8' would fail an exact-match lookup against
IP_BANLIST, allowing a banned IP to bypass the check.

Added regression tests covering padded string, array, and an
end-to-end isIpInBanlist test reproducing the reported bypass.

* fix: address CodeRabbit review feedback

- Guard against undefined rawIp when parsing an empty header array
- Delete IP_BANLIST env var in test cleanup instead of assigning undefined
- Add regression test for empty array input to parseIpFromHeaders

* test: add regression test for tab/whitespace trimming in parseIpFromHeaders

packages/lib/getIP.test.ts

@@ -1,6 +1,6 @@
 import type { NextApiRequest } from "next";
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it } from "vitest";
-import getIP, { parseIpFromHeaders } from "./getIP";
+import getIP, { isIpInBanlist, parseIpFromHeaders } from "./getIP";
 
 function buildRequest(headers: Record<string, string>): 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");
     });
   });
-});
+});

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;
-}
+}