feat: Only build payload paths on attack

+ Find multiple occurrences in same source
AikidoSec · Dec 9, 2024 · b563640 · b563640
1 parent 7c3c217
commit b563640
Show file tree

Hide file tree

Showing 27 changed files with 317 additions and 215 deletions.
diff --git a/library/agent/Agent.test.ts b/library/agent/Agent.test.ts
@@ -214,7 +214,7 @@ t.test("when attack detected", async () => {
     operation: "operation",
     payload: "payload",
     stack: "stack",
-    path: ".nested",
+    paths: [".nested"],
     metadata: {
       db: "app",
     },
@@ -277,7 +277,7 @@ t.test("it checks if user agent is a string", async () => {
     payload: "payload",
     operation: "operation",
     stack: "stack",
-    path: ".nested",
+    paths: [".nested"],
     metadata: {
       db: "app",
     },
@@ -554,7 +554,7 @@ t.test("it logs when failed to report event", async () => {
     },
     operation: "operation",
     stack: "stack",
-    path: ".nested",
+    paths: [".nested"],
     payload: "payload",
     metadata: {
       db: "app",
@@ -636,7 +636,7 @@ t.test("when payload is object", async () => {
     operation: "operation",
     payload: { $gt: "" },
     stack: "stack",
-    path: ".nested",
+    paths: [".nested"],
     metadata: {
       db: "app",
     },
@@ -664,7 +664,7 @@ t.test("when payload is object", async () => {
     operation: "operation",
     payload: "a".repeat(20000),
     stack: "stack",
-    path: ".nested",
+    paths: [".nested"],
     metadata: {
       db: "app",
     },

diff --git a/library/agent/Agent.ts b/library/agent/Agent.ts
@@ -142,7 +142,7 @@ export class Agent {
     source,
     request,
     stack,
-    path,
+    paths,
     metadata,
     payload,
   }: {
@@ -153,7 +153,7 @@ export class Agent {
     source: Source;
     request: Context;
     stack: string;
-    path: string;
+    paths: string[];
     metadata: Record<string, string>;
     payload: unknown;
   }) {
@@ -168,7 +168,7 @@ export class Agent {
               module: module,
               operation: operation,
               blocked: blocked,
-              path: path,
+              path: paths.join(),
               stack: stack,
               source: source,
               metadata: limitLengthMetadata(metadata, 4096),

diff --git a/library/agent/Context.test.ts b/library/agent/Context.test.ts
@@ -110,24 +110,24 @@ t.test("it clears cache when context is mutated", async (t) => {
     t.same(extractStringsFromUserInputCached(getContext()!, "body"), undefined);
     t.same(
       extractStringsFromUserInputCached(getContext()!, "query"),
-      new Map(Object.entries({ abc: ".", def: ".abc" }))
+      new Set(["abc", "def"])
     );
 
     updateContext(getContext()!, "query", {});
     t.same(extractStringsFromUserInputCached(getContext()!, "body"), undefined);
     t.same(
       extractStringsFromUserInputCached(getContext()!, "query"),
-      new Map(Object.entries({}))
+      new Set()
     );
 
     runWithContext({ ...context, body: { a: "z" }, query: { b: "y" } }, () => {
       t.same(
         extractStringsFromUserInputCached(getContext()!, "body"),
-        new Map(Object.entries({ a: ".", z: ".a" }))
+        new Set(["a", "z"])
       );
       t.same(
         extractStringsFromUserInputCached(getContext()!, "query"),
-        new Map(Object.entries({ b: ".", y: ".b" }))
+        new Set(["b", "y"])
       );
     });
   });

diff --git a/library/agent/applyHooks.test.ts b/library/agent/applyHooks.test.ts
@@ -161,7 +161,7 @@ t.test("it does not report attack if IP is allowed", async (t) => {
         return {
           operation: "os.hostname",
           source: "body",
-          pathToPayload: "path",
+          pathsToPayload: ["path"],
           payload: "payload",
           metadata: {},
           kind: "path_traversal",

diff --git a/library/agent/hooks/InterceptorResult.ts b/library/agent/hooks/InterceptorResult.ts
@@ -5,7 +5,7 @@ export type InterceptorResult = {
   operation: string;
   kind: Kind;
   source: Source;
-  pathToPayload: string;
+  pathsToPayload: string[];
   metadata: Record<string, string>;
   payload: unknown;
 } | void;
diff --git a/library/agent/hooks/wrapExport.ts b/library/agent/hooks/wrapExport.ts
@@ -180,15 +180,15 @@ function inspectArgs(
       source: result.source,
       blocked: agent.shouldBlock(),
       stack: cleanupStackTrace(new Error().stack!, libraryRoot),
-      path: result.pathToPayload,
+      paths: result.pathsToPayload,
       metadata: result.metadata,
       request: context,
       payload: result.payload,
     });
 
     if (agent.shouldBlock()) {
       throw new Error(
-        `Zen has blocked ${attackKindHumanName(result.kind)}: ${result.operation}(...) originating from ${result.source}${escapeHTML(result.pathToPayload)}`
+        `Zen has blocked ${attackKindHumanName(result.kind)}: ${result.operation}(...) originating from ${result.source}${escapeHTML(result.pathsToPayload.join())}`
       );
     }
   }

diff --git a/library/helpers/attackPath.test.ts b/library/helpers/attackPath.test.ts
@@ -0,0 +1,59 @@
+import * as t from "tap";
+import { getPathsToPayload as get } from "./attackPath";
+
+t.test("it gets paths to payload", async (t) => {
+  const testObj1 = {
+    a: {
+      b: {
+        c: "payload",
+      },
+    },
+    d: [12, "test", "payload"],
+  };
+
+  t.same(get("payload", testObj1), [".a.b.c", ".d.[2]"]);
+  t.same(get("test", testObj1), [".d.[1]"]);
+  t.same(get("notfound", testObj1), []);
+
+  t.same(get("payload", "payload"), ["."]);
+  t.same(get("test", "payload"), []);
+
+  t.same(get("", undefined), []);
+  t.same(get("", null), []);
+  t.same(
+    get("", () => {}),
+    []
+  );
+
+  t.same(
+    get("string", [
+      "string",
+      1,
+      true,
+      null,
+      undefined,
+      { test: "test" },
+      "string",
+    ]),
+    [".[0]", ".[6]"]
+  );
+
+  // Concatenates array values
+  t.same(get("test,test2", ["test", "test2"]), ["."]);
+  t.same(get("test,test2", { test: { x: ["test", "test2"] } }), [".test.x"]);
+});
+
+t.test("it works with jwt", async (t) => {
+  const testObj2 = {
+    a: {
+      x: ["test", "notfoundx"],
+      b: {
+        c: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.uG3R-hTeSn-DUddaexdXHw4pvXwKdyxqsD2k0BZrbd4",
+      },
+    },
+  };
+
+  t.same(get("John Doe", testObj2), [".a.b.c<jwt>.name"]);
+  t.same(get("1234567890", testObj2), [".a.b.c<jwt>.sub"]);
+  t.same(get("notfound", testObj2), []);
+});
diff --git a/library/helpers/attackPath.ts b/library/helpers/attackPath.ts
@@ -1,3 +1,6 @@
+import { isPlainObject } from "./isPlainObject";
+import { tryDecodeAsJWT } from "./tryDecodeAsJWT";
+
 export type PathPart =
   | { type: "jwt" }
   | { type: "object"; key: string }
@@ -24,3 +27,53 @@ export function buildPathToPayload(pathToPayload: PathPart[]): string {
     return acc;
   }, "");
 }
+
+export function getPathsToPayload(
+  attackPayload: string,
+  obj: unknown
+): string[] {
+  const matches: string[] = [];
+
+  const attackPayloadLowercase = attackPayload.toLowerCase();
+
+  const traverse = (value: unknown, path: PathPart[] = []) => {
+    // Handle strings
+    if (typeof value === "string") {
+      if (value.toLowerCase() === attackPayloadLowercase) {
+        matches.push(buildPathToPayload(path));
+        return;
+      }
+
+      const jwt = tryDecodeAsJWT(value);
+      if (jwt.jwt) {
+        traverse(jwt.object, path.concat({ type: "jwt" }));
+      }
+
+      return;
+    }
+
+    if (Array.isArray(value)) {
+      // Handle arrays
+      value.forEach((item, index) => {
+        traverse(item, path.concat({ type: "array", index }));
+      });
+
+      if (value.join().toLowerCase() === attackPayloadLowercase) {
+        matches.push(buildPathToPayload(path));
+      }
+
+      return;
+    }
+
+    if (isPlainObject(value)) {
+      // Handle objects
+      for (const key in value) {
+        traverse(value[key], path.concat({ type: "object", key }));
+      }
+    }
+  };
+
+  traverse(obj);
+
+  return matches;
+}