Merge branch 'main' into new-instrumentation

AikidoSec · Feb 28, 2025 · c5230b8 · c5230b8
2 parents 1992b02 + f3303e4
commit c5230b8
Show file tree

Hide file tree

Showing 11 changed files with 207 additions and 266 deletions.
diff --git a/end2end/tests/express-mongodb.ssrf.test.js b/end2end/tests/express-mongodb.ssrf.test.js
@@ -66,7 +66,7 @@ t.test("it blocks in blocking mode", (t) => {
   });
 
   server.on("error", (err) => {
-    t.fail(err.message);
+    t.fail(err);
   });
 
   let stdout = "";
@@ -92,14 +92,35 @@ t.test("it blocks in blocking mode", (t) => {
             signal: AbortSignal.timeout(5000),
           }
         ),
+        fetch(
+          `http://local.aikido.io:4000/images/${encodeURIComponent("http://local.aikido.io:4000")}`,
+          {
+            signal: AbortSignal.timeout(5000),
+            headers: {
+              Origin: "http://local.aikido.io:4000",
+              Referer: "http://local.aikido.io:4000",
+            },
+          }
+        ),
+        fetch(
+          `http://local.aikido.io:4000/images/${encodeURIComponent("http://local.aikido.io:5875")}`,
+          {
+            signal: AbortSignal.timeout(5000),
+          }
+        ),
       ]);
     })
-    .then(([safeRequest, ssrfRequest]) => {
+    .then(([safeRequest, ssrfRequest, requestToItself, differentPort]) => {
       t.equal(safeRequest.status, 200);
       t.equal(ssrfRequest.status, 500);
       t.match(stdout, /Starting agent/);
       t.match(stderr, /Zen has blocked a server-side request forgery/);
 
+      // Requests to same hostname as the server should be allowed
+      t.equal(requestToItself.status, 200);
+      // If the port is different, it should be blocked
+      t.equal(differentPort.status, 500);
+
       return fetch(`${testServerUrl}/api/runtime/events`, {
         method: "GET",
         headers: {
@@ -114,14 +135,14 @@ t.test("it blocks in blocking mode", (t) => {
       const attacks = events.filter(
         (event) => event.type === "detected_attack"
       );
-      t.same(attacks.length, 1);
+      t.same(attacks.length, 2);
       const [attack] = attacks;
       t.match(attack.attack.stack, /app\.js/);
       t.match(attack.attack.stack, /fetchImage\.js/);
       t.match(attack.attack.stack, /express-async-handler/);
     })
     .catch((error) => {
-      t.fail(error.message);
+      t.fail(error);
     })
     .finally(() => {
       server.kill();
@@ -142,6 +163,10 @@ t.test("it does not block in dry mode", (t) => {
     t.end();
   });
 
+  server.on("error", (err) => {
+    t.fail(err);
+  });
+
   let stdout = "";
   server.stdout.on("data", (data) => {
     stdout += data.toString();
@@ -174,7 +199,58 @@ t.test("it does not block in dry mode", (t) => {
       t.notMatch(stderr, /Zen has blocked a server-side request forgery/);
     })
     .catch((error) => {
-      t.fail(error.message);
+      t.fail(error);
+    })
+    .finally(() => {
+      server.kill();
+    });
+});
+
+t.test("it blocks request to base URL if proxy is not trusted", (t) => {
+  const server = spawn(`node`, [pathToApp, "4002"], {
+    env: {
+      ...process.env,
+      AIKIDO_DEBUG: "true",
+      AIKIDO_BLOCKING: "true",
+      AIKIDO_TOKEN: token,
+      AIKIDO_URL: testServerUrl,
+      AIKIDO_TRUST_PROXY: "false",
+    },
+  });
+
+  server.on("close", () => {
+    t.end();
+  });
+
+  server.on("error", (err) => {
+    t.fail(err);
+  });
+
+  let stdout = "";
+  server.stdout.on("data", (data) => {
+    stdout += data.toString();
+  });
+
+  let stderr = "";
+  server.stderr.on("data", (data) => {
+    stderr += data.toString();
+  });
+
+  // Wait for the server to start
+  timeout(2000)
+    .then(() => {
+      return fetch(
+        `http://local.aikido.io:4002/images/${encodeURIComponent("http://local.aikido.io:4002")}`,
+        {
+          signal: AbortSignal.timeout(5000),
+        }
+      );
+    })
+    .then((requestToItself) => {
+      t.equal(requestToItself.status, 500);
+    })
+    .catch((error) => {
+      t.fail(error);
     })
     .finally(() => {
       server.kill();

diff --git a/library/helpers/getIPAddressFromRequest.ts b/library/helpers/getIPAddressFromRequest.ts
@@ -1,5 +1,6 @@
 import { isIP } from "net";
 import { isPrivateIP } from "../vulnerabilities/ssrf/isPrivateIP";
+import { trustProxy } from "./trustProxy";
 
 export function getIPAddressFromRequest(req: {
   headers: Record<string, unknown>;
@@ -57,16 +58,3 @@ function getClientIpFromXForwardedFor(value: string) {
 
   return null;
 }
-
-function trustProxy() {
-  if (!process.env.AIKIDO_TRUST_PROXY) {
-    // Trust proxy by default
-    // Most of the time, the application is behind a reverse proxy
-    return true;
-  }
-
-  return (
-    process.env.AIKIDO_TRUST_PROXY === "1" ||
-    process.env.AIKIDO_TRUST_PROXY === "true"
-  );
-}
diff --git a/library/helpers/trustProxy.test.ts b/library/helpers/trustProxy.test.ts
@@ -0,0 +1,20 @@
+import * as t from "tap";
+import { trustProxy } from "./trustProxy";
+
+t.beforeEach(() => {
+  delete process.env.AIKIDO_TRUST_PROXY;
+});
+
+t.test("the default is true", async () => {
+  t.equal(trustProxy(), true);
+});
+
+t.test("trust proxy set to false", async () => {
+  process.env.AIKIDO_TRUST_PROXY = "false";
+  t.equal(trustProxy(), false);
+});
+
+t.test("trust proxy set to true", async () => {
+  process.env.AIKIDO_TRUST_PROXY = "true";
+  t.equal(trustProxy(), true);
+});
diff --git a/library/helpers/trustProxy.ts b/library/helpers/trustProxy.ts
@@ -0,0 +1,11 @@
+import { envToBool } from "./envToBool";
+
+export function trustProxy() {
+  if (!process.env.AIKIDO_TRUST_PROXY) {
+    // Trust proxy by default
+    // Most of the time, the application is behind a reverse proxy
+    return true;
+  }
+
+  return envToBool(process.env.AIKIDO_TRUST_PROXY);
+}
diff --git a/library/sinks/Fetch.localhost.test.ts b/library/sinks/Fetch.localhost.test.ts
@@ -1,11 +1,8 @@
 /* eslint-disable prefer-rest-params */
 import * as t from "tap";
-import { Agent } from "../agent/Agent";
 import { createServer, Server } from "http";
-import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
 import { Token } from "../agent/api/Token";
 import { Context, runWithContext } from "../agent/Context";
-import { LoggerNoop } from "../agent/logger/LoggerNoop";
 import { createTestAgent } from "../helpers/createTestAgent";
 import { Fetch } from "./Fetch";
 

diff --git a/library/sinks/Fetch.test.ts b/library/sinks/Fetch.test.ts
@@ -43,20 +43,22 @@ wrap(dns, "lookup", function lookup(original) {
   };
 });
 
-const context: Context = {
-  remoteAddress: "::1",
-  method: "POST",
-  url: "http://localhost:4000",
-  query: {},
-  headers: {},
-  body: {
-    image: "http://localhost:4000/api/internal",
-  },
-  cookies: {},
-  routeParams: {},
-  source: "express",
-  route: "/posts/:id",
-};
+function createContext(): Context {
+  return {
+    remoteAddress: "::1",
+    method: "POST",
+    url: "http://local.aikido.io:4000",
+    query: {},
+    headers: {},
+    body: {
+      image: "http://localhost:4000/api/internal",
+    },
+    cookies: {},
+    routeParams: {},
+    source: "express",
+    route: "/posts/:id",
+  };
+}
 
 const redirectTestUrl = "http://ssrf-redirects.testssandbox.com";
 const redirecTestUrl2 =
@@ -113,7 +115,7 @@ t.test(
 
     agent.getHostnames().clear();
 
-    await runWithContext(context, async () => {
+    await runWithContext(createContext(), async () => {
       // Don't await fetch to see how it handles
       // multiple requests at the same time
       // Because there's a single instance of the dispatcher
@@ -178,7 +180,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{
           body: {
             image2: [
@@ -221,7 +223,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.ip } },
       },
       async () => {
@@ -238,7 +240,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.domain } },
       },
       async () => {
@@ -255,7 +257,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.ipTwice } },
       },
       async () => {
@@ -272,7 +274,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.domainTwice } },
       },
       async () => {
@@ -291,7 +293,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.ipv6 } },
       },
       async () => {
@@ -308,7 +310,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.ipv6Twice } },
       },
       async () => {
@@ -325,7 +327,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{
           body: {
             image: `${redirecTestUrl2}/ssrf-test-absolute-domain`,
@@ -349,7 +351,7 @@ t.test(
     // Manual redirect
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.ip } },
       },
       async () => {
@@ -373,7 +375,7 @@ t.test(
     // Manual redirect
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{ body: { image: redirectUrl.domain } },
       },
       async () => {
@@ -398,7 +400,7 @@ t.test(
 
     await runWithContext(
       {
-        ...context,
+        ...createContext(),
         ...{
           body: {
             image: `${redirecTestUrl2}/ssrf-test-absolute-domain`,