Skip to content

Commit

Permalink
Merge pull request #484 from AikidoSec/clean-stacktraces
Browse files Browse the repository at this point in the history
Clean up error stack traces on detected attack
  • Loading branch information
bitterpanda63 authored Dec 23, 2024
2 parents 64d67c7 + 640294d commit 000d17f
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 11 deletions.
7 changes: 5 additions & 2 deletions library/agent/hooks/onInspectionInterceptorResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "../..");
Expand Down Expand Up @@ -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())}`
)
);
}
}
Expand Down
19 changes: 19 additions & 0 deletions library/helpers/cleanError.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
12 changes: 12 additions & 0 deletions library/helpers/cleanError.ts
Original file line number Diff line number Diff line change
@@ -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;
}
7 changes: 7 additions & 0 deletions library/helpers/getLibraryRoot.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { resolve } from "path";

const libraryRoot = resolve(__dirname, "..");

export function getLibraryRoot(): string {
return libraryRoot;
}
1 change: 1 addition & 0 deletions library/sinks/FileSystem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() =>
Expand Down
11 changes: 8 additions & 3 deletions library/sinks/undici/wrapDispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"];

Expand Down Expand Up @@ -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,
Expand All @@ -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())}`
)
);
}
}
Expand Down
13 changes: 7 additions & 6 deletions library/vulnerabilities/ssrf/inspectDNSLookupCalls.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -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())}`
)
)
);
}
Expand Down

0 comments on commit 000d17f

Please sign in to comment.