Merge pull request #158 from AikidoSec/AIK-2590

Protect against SSRF attacks

Author: hansott

README.md

@@ -22,6 +22,7 @@ Firewall autonomously protects your Node.js applications against:
 * 🛡️ [Command injection attacks](https://owasp.org/www-community/attacks/Command_Injection)
 * 🛡️ [Prototype pollution](./docs/prototype-pollution.md)
 * 🛡️ [Path traversal attacks](https://owasp.org/www-community/attacks/Path_Traversal)
+* 🛡️ [Server-side request forgery (SSRF)](./docs/ssrf.md)
 * 🚀 More to come (see the [public roadmap](https://github.com/orgs/AikidoSec/projects/2/views/1))!
 
 Firewall operates autonomously on the same server as your Node.js app to:

docs/ssrf.md

@@ -0,0 +1,31 @@
+# Server-side request forgery (SSRF)
+
+Aikido firewall for Node.js 16+ secures your app against server-side request forgery (SSRF) attacks. SSRF vulnerabilities allow attackers to send crafted requests to internal services, bypassing firewalls and security controls. Runtime blocks SSRF attacks by intercepting and validating requests to internal services.
+
+## Example
+
+```
+GET https://your-app.com/files?url=http://localhost:3000/private
+```
+
+```js
+const response = http.request(req.query.url);
+```
+
+In this example, an attacker sends a request to `localhost:3000/private` from your server. Firewall can intercept the request and block it, preventing the attacker from accessing internal services.
+
+```
+GET https://your-app.com/files?url=http://localtest.me:3000/private
+```
+
+In this example, the attacker sends a request to `localtest.me:3000/private`, which resolves to `127.0.0.1`. Firewall can intercept the request and block it, preventing the attacker from accessing internal services.
+
+We don't protect against stored SSRF attacks, where an attacker injects a malicious URL into your app's database. To prevent stored SSRF attacks, validate and sanitize user input before storing it in your database.
+
+## Which built-in modules are protected?
+
+Firewall protects against SSRF attacks in the following built-in modules:
+* `http`
+* `https`
+* `undici`
+* `globalThis.fetch` (Node.js 18+)

library/agent/Agent.ts

@@ -425,6 +425,10 @@ export class Agent {
     return this.routes;
   }
 
+  log(message: string) {
+    this.logger.log(message);
+  }
+
   async flushStats(timeoutInMS: number) {
     this.statistics.forceCompress();
     await this.sendHeartbeat(timeoutInMS);

library/agent/Attack.ts

@@ -2,7 +2,8 @@ export type Kind =
   | "nosql_injection"
   | "sql_injection"
   | "shell_injection"
-  | "path_traversal";
+  | "path_traversal"
+  | "ssrf";
 
 export function attackKindHumanName(kind: Kind) {
   switch (kind) {
@@ -14,5 +15,7 @@ export function attackKindHumanName(kind: Kind) {
       return "a shell injection";
     case "path_traversal":
       return "a path traversal attack";
+    case "ssrf":
+      return "a server-side request forgery";
   }
 }

library/agent/applyHooks.test.ts

@@ -94,9 +94,9 @@ t.test("it tries to wrap method that does not exist", async (t) => {
   });
 
   t.same(logger.getMessages(), [
-    "Failed to wrap method does_not_exist in module shell-quote",
-    "Failed to wrap method another_method_that_does_not_exist in module shell-quote",
     "Failed to wrap method another_second_method_that_does_not_exist in module shell-quote",
+    "Failed to wrap method another_method_that_does_not_exist in module shell-quote",
+    "Failed to wrap method does_not_exist in module shell-quote",
   ]);
 });
 

library/agent/applyHooks.ts

@@ -92,17 +92,15 @@ export function applyHooks(hooks: Hooks, agent: Agent) {
       return;
     }
 
-    const interceptor = g.getMethodInterceptor();
-
-    if (!interceptor) {
-      return;
-    }
-
-    if (interceptor instanceof ModifyingArgumentsMethodInterceptor) {
-      wrapWithArgumentModification(global, interceptor, "global", agent);
-    } else {
-      wrapWithoutArgumentModification(global, interceptor, "global", agent);
-    }
+    g.getMethodInterceptors()
+      .reverse() // Reverse to make sure we wrap in the order they were added
+      .forEach((interceptor) => {
+        if (interceptor instanceof ModifyingArgumentsMethodInterceptor) {
+          wrapWithArgumentModification(global, interceptor, name, agent);
+        } else {
+          wrapWithoutArgumentModification(global, interceptor, name, agent);
+        }
+      });
   });
 
   return wrapped;
@@ -364,15 +362,18 @@ function wrapSubject(
     return;
   }
 
-  subject.getMethodInterceptors().forEach((method) => {
-    if (method instanceof ModifyingArgumentsMethodInterceptor) {
-      wrapWithArgumentModification(theSubject, method, module, agent);
-    } else if (method instanceof MethodInterceptor) {
-      wrapWithoutArgumentModification(theSubject, method, module, agent);
-    } else if (method instanceof MethodResultInterceptor) {
-      wrapWithResult(theSubject, method, module, agent);
-    } else {
-      wrapNewInstance(theSubject, method, module, agent);
-    }
-  });
+  subject
+    .getMethodInterceptors()
+    .reverse() // Reverse to make sure we wrap in the order they were added
+    .forEach((method) => {
+      if (method instanceof ModifyingArgumentsMethodInterceptor) {
+        wrapWithArgumentModification(theSubject, method, module, agent);
+      } else if (method instanceof MethodInterceptor) {
+        wrapWithoutArgumentModification(theSubject, method, module, agent);
+      } else if (method instanceof MethodResultInterceptor) {
+        wrapWithResult(theSubject, method, module, agent);
+      } else {
+        wrapNewInstance(theSubject, method, module, agent);
+      }
+    });
 }

library/agent/hooks/Global.ts

@@ -5,10 +5,8 @@ import {
 } from "./ModifyingArgumentsInterceptor";
 
 export class Global {
-  private method:
-    | MethodInterceptor
-    | ModifyingArgumentsMethodInterceptor
-    | undefined = undefined;
+  private methods: (MethodInterceptor | ModifyingArgumentsMethodInterceptor)[] =
+    [];
 
   constructor(private readonly name: string) {
     if (!this.name) {
@@ -22,7 +20,8 @@ export class Global {
    * This is the preferred way to use when wrapping methods
    */
   inspect(interceptor: Interceptor) {
-    this.method = new MethodInterceptor(this.name, interceptor);
+    const method = new MethodInterceptor(this.name, interceptor);
+    this.methods.push(method);
 
     return this;
   }
@@ -35,10 +34,11 @@ export class Global {
    * Don't use this unless you have to, it's better to use inspect
    */
   modifyArguments(interceptor: ModifyingArgumentsInterceptor) {
-    this.method = new ModifyingArgumentsMethodInterceptor(
+    const method = new ModifyingArgumentsMethodInterceptor(
       this.name,
       interceptor
     );
+    this.methods.push(method);
 
     return this;
   }
@@ -47,7 +47,7 @@ export class Global {
     return this.name;
   }
 
-  getMethodInterceptor() {
-    return this.method;
+  getMethodInterceptors() {
+    return this.methods;
   }
 }

library/agent/logger/LoggerForTesting.ts

@@ -1,7 +1,7 @@
 import { Logger } from "./Logger";
 
 export class LoggerForTesting implements Logger {
-  private readonly messages: string[] = [];
+  private messages: string[] = [];
 
   log(message: string) {
     this.messages.push(message);
@@ -10,4 +10,8 @@ export class LoggerForTesting implements Logger {
   getMessages() {
     return this.messages;
   }
+
+  clear() {
+    this.messages = [];
+  }
 }

library/helpers/tryParseURL.ts

@@ -1,4 +1,4 @@
-export function tryParseURL(url: string) {
+export function tryParseURL(url: string): URL | undefined {
   try {
     return new URL(url);
   } catch {

library/sinks/Fetch.test.ts

@@ -1,9 +1,49 @@
+/* eslint-disable prefer-rest-params */
 import * as t from "tap";
 import { Agent } from "../agent/Agent";
 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 { wrap } from "../helpers/wrap";
 import { Fetch } from "./Fetch";
+import * as dns from "dns";
+
+const calls: Record<string, number> = {};
+wrap(dns, "lookup", function lookup(original) {
+  return function lookup() {
+    const hostname = arguments[0];
+
+    if (!calls[hostname]) {
+      calls[hostname] = 0;
+    }
+
+    calls[hostname]++;
+
+    if (hostname === "thisdomainpointstointernalip.com") {
+      return original.apply(this, [
+        "localhost",
+        ...Array.from(arguments).slice(1),
+      ]);
+    }
+
+    original.apply(this, arguments);
+  };
+});
+
+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",
+};
 
 t.test(
   "it works",
@@ -13,7 +53,7 @@ t.test(
       true,
       new LoggerNoop(),
       new ReportingAPIForTesting(),
-      new Token("123"),
+      undefined,
       undefined
     );
     agent.start([new Fetch()]);
@@ -39,5 +79,51 @@ t.test(
 
     t.same(agent.getHostnames().asArray(), []);
     agent.getHostnames().clear();
+
+    await runWithContext(context, async () => {
+      await fetch("https://google.com");
+      const error = await t.rejects(() =>
+        fetch("http://localhost:4000/api/internal")
+      );
+      if (error instanceof Error) {
+        t.same(
+          error.message,
+          "Aikido firewall has blocked a server-side request forgery: fetch(...) originating from body.image"
+        );
+      }
+
+      const error2 = await t.rejects(() =>
+        fetch(new URL("http://localhost:4000/api/internal"))
+      );
+      if (error2 instanceof Error) {
+        t.same(
+          error2.message,
+          "Aikido firewall has blocked a server-side request forgery: fetch(...) originating from body.image"
+        );
+      }
+    });
+
+    await runWithContext(
+      {
+        ...context,
+        ...{ body: { image: "http://thisdomainpointstointernalip.com" } },
+      },
+      async () => {
+        const error = await t.rejects(() =>
+          fetch("http://thisdomainpointstointernalip.com")
+        );
+        if (error instanceof Error) {
+          t.same(
+            // @ts-expect-error Type is not defined
+            error.cause.message,
+            "Aikido firewall has blocked a server-side request forgery: fetch(...) originating from body.image"
+          );
+        }
+
+        // Ensure the lookup is only called once per hostname
+        // Otherwise, it could be vulnerable to TOCTOU
+        t.same(calls["thisdomainpointstointernalip.com"], 1);
+      }
+    );
   }
 );