Merge pull request #484 from AikidoSec/clean-stacktraces

Clean up error stack traces on detected attack

Author: bitterpanda63

library/agent/hooks/onInspectionInterceptorResult.ts

@@ -6,6 +6,7 @@ import { attackKindHumanName } from "../Attack";
 import { getContext, updateContext } from "../Context";
 import type { InterceptorResult } from "./InterceptorResult";
 import type { WrapPackageInfo } from "./WrapPackageInfo";
+import { cleanError } from "../../helpers/cleanError";
 
 // Used for cleaning up the stack trace
 const libraryRoot = resolve(__dirname, "../..");
@@ -49,8 +50,10 @@ export function onInspectionInterceptorResult(
     });
 
     if (agent.shouldBlock()) {
-      throw new Error(
-        `Zen has blocked ${attackKindHumanName(result.kind)}: ${result.operation}(...) originating from ${result.source}${escapeHTML((result.pathsToPayload || []).join())}`
+      throw cleanError(
+        new Error(
+          `Zen has blocked ${attackKindHumanName(result.kind)}: ${result.operation}(...) originating from ${result.source}${escapeHTML((result.pathsToPayload || []).join())}`
+        )
       );
     }
   }

library/helpers/cleanError.test.ts

@@ -0,0 +1,19 @@
+import * as t from "tap";
+import { cleanError } from "./cleanError";
+
+t.test("it works", async () => {
+  const error = new Error("test");
+  t.same(error.message, "test");
+  t.same(error.name, "Error");
+  t.same(error.stack!.includes("cleanError.test.ts"), true);
+
+  const cleaned = cleanError(new Error("test"));
+  t.same(cleaned.message, "test");
+  t.same(cleaned.name, "Error");
+  t.same(cleaned.stack!.includes("cleanError.test.ts"), false);
+
+  const cleaned2 = cleanError(new TypeError("test2"));
+  t.same(cleaned2.message, "test2");
+  t.same(cleaned2.name, "TypeError");
+  t.same(cleaned2.stack!.includes("cleanError.test.ts"), false);
+});

library/helpers/cleanError.ts

@@ -0,0 +1,12 @@
+import { cleanupStackTrace } from "./cleanupStackTrace";
+import { getLibraryRoot } from "./getLibraryRoot";
+
+// Cleans up the error stack trace by removing all the lines that are part of the library.
+// e.g. useful to hide the module patching if we throw an error on a detected attack.
+export function cleanError(err: Error) {
+  if (err.stack) {
+    err.stack = cleanupStackTrace(err.stack, getLibraryRoot());
+  }
+
+  return err;
+}

library/helpers/getLibraryRoot.ts

@@ -0,0 +1,7 @@
+import { resolve } from "path";
+
+const libraryRoot = resolve(__dirname, "..");
+
+export function getLibraryRoot(): string {
+  return libraryRoot;
+}

library/sinks/FileSystem.test.ts

@@ -140,6 +140,7 @@ t.test("it works", async (t) => {
         error.message,
         "Zen has blocked a path traversal attack: fs.writeFile(...) originating from body.file.matches"
       );
+      t.same(error.stack!.includes("wrapExport.ts"), false);
     }
 
     const error2 = await t.rejects(() =>

library/sinks/undici/wrapDispatch.ts

@@ -9,6 +9,9 @@ import { attackKindHumanName } from "../../agent/Attack";
 import { escapeHTML } from "../../helpers/escapeHTML";
 import { isRedirectToPrivateIP } from "../../vulnerabilities/ssrf/isRedirectToPrivateIP";
 import { wrapOnHeaders } from "./wrapOnHeaders";
+import { cleanError } from "../../helpers/cleanError";
+import { cleanupStackTrace } from "../../helpers/cleanupStackTrace";
+import { getLibraryRoot } from "../../helpers/getLibraryRoot";
 
 type Dispatch = Dispatcher["dispatch"];
 
@@ -98,7 +101,7 @@ function blockRedirectToPrivateIP(url: URL, context: Context, agent: Agent) {
       kind: "ssrf",
       source: found.source,
       blocked: agent.shouldBlock(),
-      stack: new Error().stack!,
+      stack: cleanupStackTrace(new Error().stack!, getLibraryRoot()),
       paths: found.pathsToPayload,
       metadata: getMetadataForSSRFAttack({
         hostname: found.hostname,
@@ -109,8 +112,10 @@ function blockRedirectToPrivateIP(url: URL, context: Context, agent: Agent) {
     });
 
     if (agent.shouldBlock()) {
-      throw new Error(
-        `Zen has blocked ${attackKindHumanName("ssrf")}: fetch(...) originating from ${found.source}${escapeHTML((found.pathsToPayload || []).join())}`
+      throw cleanError(
+        new Error(
+          `Zen has blocked ${attackKindHumanName("ssrf")}: fetch(...) originating from ${found.source}${escapeHTML((found.pathsToPayload || []).join())}`
+        )
       );
     }
   }

library/vulnerabilities/ssrf/inspectDNSLookupCalls.ts

@@ -1,6 +1,5 @@
 import { isIP, type LookupFunction } from "net";
 import { LookupAddress } from "dns";
-import { resolve } from "path";
 import { Agent } from "../../agent/Agent";
 import { attackKindHumanName } from "../../agent/Attack";
 import { getContext } from "../../agent/Context";
@@ -14,6 +13,8 @@ import { RequestContextStorage } from "../../sinks/undici/RequestContextStorage"
 import { findHostnameInContext } from "./findHostnameInContext";
 import { getRedirectOrigin } from "./getRedirectOrigin";
 import { getPortFromURL } from "../../helpers/getPortFromURL";
+import { getLibraryRoot } from "../../helpers/getLibraryRoot";
+import { cleanError } from "../../helpers/cleanError";
 
 export function inspectDNSLookupCalls(
   lookup: Function,
@@ -189,8 +190,6 @@ function wrapDNSLookupCallback(
       return callback(err, addresses, family);
     }
 
-    const libraryRoot = resolve(__dirname, "../..");
-
     // Used to get the stack trace of the calling location
     // We don't throw the error, we just use it to get the stack trace
     const stackTraceError = callingLocationStackTrace || new Error();
@@ -201,7 +200,7 @@ function wrapDNSLookupCallback(
       kind: "ssrf",
       source: found.source,
       blocked: agent.shouldBlock(),
-      stack: cleanupStackTrace(stackTraceError.stack!, libraryRoot),
+      stack: cleanupStackTrace(stackTraceError.stack!, getLibraryRoot()),
       paths: found.pathsToPayload,
       metadata: getMetadataForSSRFAttack({ hostname, port }),
       request: context,
@@ -210,8 +209,10 @@ function wrapDNSLookupCallback(
 
     if (agent.shouldBlock()) {
       return callback(
-        new Error(
-          `Zen has blocked ${attackKindHumanName("ssrf")}: ${operation}(...) originating from ${found.source}${escapeHTML((found.pathsToPayload || []).join())}`
+        cleanError(
+          new Error(
+            `Zen has blocked ${attackKindHumanName("ssrf")}: ${operation}(...) originating from ${found.source}${escapeHTML((found.pathsToPayload || []).join())}`
+          )
         )
       );
     }