Merge pull request #479 from AikidoSec/patch-attack-path

Fix `getPathsToPayload` not respecting max matches
AikidoSec · Dec 18, 2024 · a892b72 · a892b72
2 parents c9cc7a5 + d1917f7
commit a892b72
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 21 deletions.
diff --git a/library/helpers/attackPath.test.ts b/library/helpers/attackPath.test.ts
@@ -1,6 +1,11 @@
 import * as t from "tap";
 import { getPathsToPayload as get } from "./attackPath";
 
+t.test("it throws error if max matches is less than 1", async (t) => {
+  t.throws(() => get("payload", {}, 0));
+  t.throws(() => get("payload", {}, -1));
+});
+
 t.test("it gets paths to payload", async (t) => {
   const testObj1 = {
     a: {
@@ -87,3 +92,19 @@ t.test("respects max depth and array length", async (t) => {
   t.same(get("100", testArr), [".[100]"]);
   t.same(get("101", testArr), []);
 });
+
+t.test("first item in array", async (t) => {
+  t.same(get("id = 1", ["id = 1"]), [".[0]"]);
+});
+
+t.test("it checks max matches when iterating over object props", async (t) => {
+  const testObj = {
+    a: ["test"],
+    b: ["test"],
+    c: ["test"],
+  };
+
+  t.same(get("test", testObj, 1), [".a.[0]"]);
+  t.same(get("test", testObj, 2), [".a.[0]", ".b.[0]"]);
+  t.same(get("test", testObj, 3), [".a.[0]", ".b.[0]", ".c.[0]"]);
+});
diff --git a/library/helpers/attackPath.ts b/library/helpers/attackPath.ts
@@ -37,64 +37,84 @@ export function buildPathToPayload(pathToPayload: PathPart[]): string {
   }, "");
 }
 
+class Matches {
+  private readonly matches: string[] = [];
+
+  constructor(private readonly max: number) {
+    if (max < 1) {
+      throw new Error("Max must be greater than 0");
+    }
+  }
+
+  add(path: PathPart[]) {
+    this.matches.push(buildPathToPayload(path));
+  }
+
+  getMatches() {
+    return this.matches;
+  }
+
+  found() {
+    return this.matches.length >= this.max;
+  }
+}
+
 export function getPathsToPayload(
   attackPayload: string,
   obj: unknown,
   matchCount = DEFAULT_MATCH_COUNT
 ): string[] {
-  const matches: string[] = [];
-
+  const matches = new Matches(matchCount);
   const attackPayloadLowercase = attackPayload.toLowerCase();
 
   const traverse = (value: unknown, path: PathPart[] = [], depth = 0) => {
-    if (matches.length >= matchCount) {
-      return;
-    }
-
-    if (depth > MAX_DEPTH) {
+    if (matches.found() || depth > MAX_DEPTH) {
       return;
     }
 
-    // Handle strings
     if (typeof value === "string") {
       if (value.toLowerCase() === attackPayloadLowercase) {
-        matches.push(buildPathToPayload(path));
+        matches.add(path);
         return;
       }
 
       const jwt = tryDecodeAsJWT(value);
       if (jwt.jwt) {
         traverse(jwt.object, path.concat({ type: "jwt" }), depth + 1);
       }
-
-      return;
     }
 
     if (Array.isArray(value)) {
-      // Handle arrays
+      if (
+        value.length > 1 &&
+        value.length < MAX_ARRAY_LENGTH &&
+        value.join().toLowerCase() === attackPayloadLowercase
+      ) {
+        matches.add(path);
+        return;
+      }
+
       for (const [index, item] of value.entries()) {
-        if (index > MAX_ARRAY_LENGTH) {
+        if (matches.found() || index > MAX_ARRAY_LENGTH) {
           break;
         }
-        traverse(item, path.concat({ type: "array", index }), depth);
-      }
 
-      if (value.join().toLowerCase() === attackPayloadLowercase) {
-        matches.push(buildPathToPayload(path));
+        traverse(item, path.concat({ type: "array", index }), depth);
       }
-
-      return;
     }
 
     if (isPlainObject(value)) {
-      // Handle objects
       for (const key in value) {
+        if (matches.found()) {
+          break;
+        }
+
         traverse(value[key], path.concat({ type: "object", key }), depth + 1);
       }
     }
   };
 
   traverse(obj);
 
-  return matches;
+  return matches.getMatches();
 }