Select the first valid & non-private IP from x-forwarded-for header (#…

…519)

* Select the first valid & non-private IP from x-forwarded-for header

* Add test case for IP with port

* More tests

* Fix unit test

* Fix test

* Revert "Speed up unit tests (#497)"

This reverts commit 3005698.

end2end/tests/hono-xml-blocklists.test.js

@@ -44,7 +44,7 @@ t.beforeEach(async () => {
       Authorization: token,
     },
     body: JSON.stringify({
-      blockedIPAddresses: ["1.3.2.0/24", "fe80::1234:5678:abcd:ef12/64"],
+      blockedIPAddresses: ["1.3.2.0/24", "e98c:a7ba:2329:8c69::/64"],
       blockedUserAgents: "hacker|attacker|GPTBot",
     }),
   });
@@ -98,19 +98,37 @@ t.test("it blocks geo restricted IPs", (t) => {
         "Your IP address is blocked due to geo restrictions. (Your IP: 1.3.2.4)"
       );
 
+      const xForwardedForWithPrivateIP = await fetch(
+        "http://127.0.0.1:4002/add",
+        {
+          method: "POST",
+          body: "<cat><name>Njuska</name></cat>",
+          headers: {
+            "Content-Type": "application/xml",
+            "X-Forwarded-For": "127.0.0.1, 1.3.2.4",
+          },
+          signal: AbortSignal.timeout(5000),
+        }
+      );
+      t.same(xForwardedForWithPrivateIP.status, 403);
+      t.same(
+        await xForwardedForWithPrivateIP.text(),
+        "Your IP address is blocked due to geo restrictions. (Your IP: 1.3.2.4)"
+      );
+
       const resp2 = await fetch("http://127.0.0.1:4002/add", {
         method: "POST",
         body: "<cat><name>Harry</name></cat>",
         headers: {
           "Content-Type": "application/xml",
-          "X-Forwarded-For": "fe80::1234:5678:abcd:ef12",
+          "X-Forwarded-For": "e98c:a7ba:2329:8c69:a13a:8aff:a932:13f2",
         },
         signal: AbortSignal.timeout(5000),
       });
       t.same(resp2.status, 403);
       t.same(
         await resp2.text(),
-        "Your IP address is blocked due to geo restrictions. (Your IP: fe80::1234:5678:abcd:ef12)"
+        "Your IP address is blocked due to geo restrictions. (Your IP: e98c:a7ba:2329:8c69:a13a:8aff:a932:13f2)"
       );
 
       const resp3 = await fetch("http://127.0.0.1:4002/add", {

library/helpers/getIPAddressFromRequest.test.ts

@@ -0,0 +1,280 @@
+import * as t from "tap";
+import { getIPAddressFromRequest } from "./getIPAddressFromRequest";
+
+t.beforeEach(() => {
+  delete process.env.AIKIDO_TRUST_PROXY;
+});
+
+t.test("no headers and no remote address", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "false";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {},
+      remoteAddress: undefined,
+    }),
+    undefined
+  );
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {},
+      remoteAddress: undefined,
+    }),
+    undefined
+  );
+});
+
+t.test("no headers and remote address", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "false";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {},
+      remoteAddress: "1.2.3.4",
+    }),
+    "1.2.3.4"
+  );
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {},
+      remoteAddress: "1.2.3.4",
+    }),
+    "1.2.3.4"
+  );
+});
+
+t.test("x-forwarded-for without trust proxy", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "false";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "9.9.9.9",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "1.2.3.4"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880",
+      },
+      remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+    }),
+    "df89:84af:85e0:c55f:960c:341a:2cc6:734d"
+  );
+});
+
+t.test(
+  'x-forwarded-for with trust proxy and "x-forwarded-for" is not an IP',
+  async (t) => {
+    process.env.AIKIDO_TRUST_PROXY = "true";
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "invalid",
+        },
+        remoteAddress: "1.2.3.4",
+      }),
+      "1.2.3.4"
+    );
+  }
+);
+
+t.test("x-forwarded-for with trust proxy and IP contains port", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "9.9.9.9:8080",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+});
+
+t.test("with trailing comma", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "9.9.9.9,",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": ",9.9.9.9",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": ",9.9.9.9,",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": ",9.9.9.9,,",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+});
+
+t.test(
+  'x-forwarded-for with trust proxy and "x-forwarded-for" is a private IP',
+  async (t) => {
+    process.env.AIKIDO_TRUST_PROXY = "true";
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "127.0.0.1",
+        },
+        remoteAddress: "1.2.3.4",
+      }),
+      "1.2.3.4"
+    );
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "::1",
+        },
+        remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+      }),
+      "df89:84af:85e0:c55f:960c:341a:2cc6:734d"
+    );
+  }
+);
+
+t.test(
+  'x-forwarded-for with trust proxy and "x-forwarded-for" contains private IP',
+  async (t) => {
+    process.env.AIKIDO_TRUST_PROXY = "true";
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "127.0.0.1, 9.9.9.9",
+        },
+        remoteAddress: "1.2.3.4",
+      }),
+      "9.9.9.9"
+    );
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "::1, a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880",
+        },
+        remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+      }),
+      "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880"
+    );
+  }
+);
+
+t.test(
+  'x-forwarded-for with trust proxy and "x-forwarded-for" is public IP',
+  async (t) => {
+    process.env.AIKIDO_TRUST_PROXY = "true";
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "9.9.9.9",
+        },
+        remoteAddress: "1.2.3.4",
+      }),
+      "9.9.9.9"
+    );
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880",
+        },
+        remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+      }),
+      "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880"
+    );
+  }
+);
+
+t.test(
+  'x-forwarded-for with trust proxy and "x-forwarded-for"  contains private IP at the end',
+  async (t) => {
+    process.env.AIKIDO_TRUST_PROXY = "true";
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "9.9.9.9, 127.0.0.1",
+        },
+        remoteAddress: "1.2.3.4",
+      }),
+      "9.9.9.9"
+    );
+    t.same(
+      getIPAddressFromRequest({
+        headers: {
+          "x-forwarded-for": "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880, ::1",
+        },
+        remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+      }),
+      "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880"
+    );
+  }
+);
+
+t.test("x-forwarded-for with trust proxy and multiple IPs", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "9.9.9.9, 8.8.8.8, 7.7.7.7",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for":
+          "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880, 3b07:2fba:0270:2149:5fc1:2049:5f04:2131, 791d:967e:428a:90b9:8f6f:4fcc:5d88:015d",
+      },
+      remoteAddress: "df89:84af:85e0:c55f:960c:341a:2cc6:734d",
+    }),
+    "a3ad:8f95:d2a8:454b:cf19:be6e:73c6:f880"
+  );
+});
+
+t.test("x-forwarded-for with trust proxy and many IPs", async (t) => {
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "127.0.0.1, 192.168.0.1, 192.168.0.2, 9.9.9.9",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+  t.same(
+    getIPAddressFromRequest({
+      headers: {
+        "x-forwarded-for": "9.9.9.9, 127.0.0.1, 192.168.0.1, 192.168.0.2",
+      },
+      remoteAddress: "1.2.3.4",
+    }),
+    "9.9.9.9"
+  );
+});

library/helpers/getIPAddressFromRequest.ts

@@ -1,4 +1,5 @@
 import { isIP } from "net";
+import { isPrivateIP } from "../vulnerabilities/ssrf/isPrivateIP";
 
 export function getIPAddressFromRequest(req: {
   headers: Record<string, unknown>;
@@ -27,6 +28,10 @@ function getClientIpFromXForwardedFor(value: string) {
   const forwardedIps = value.split(",").map((e) => {
     const ip = e.trim();
 
+    if (isIP(ip)) {
+      return ip;
+    }
+
     if (ip.includes(":")) {
       const parts = ip.split(":");
 
@@ -38,8 +43,10 @@ function getClientIpFromXForwardedFor(value: string) {
     return ip;
   });
 
+  // When selecting an address from the X-Forwarded-For header,
+  // we should select the first valid IP address that is not a private IP address
   for (let i = 0; i < forwardedIps.length; i++) {
-    if (isIP(forwardedIps[i])) {
+    if (isIP(forwardedIps[i]) && !isPrivateIP(forwardedIps[i])) {
       return forwardedIps[i];
     }
   }