Merge pull request #1016 from AikidoSec/fix-call-stack-size

fix: Possible to reach max call stack size

Author: hansott

library/agent/api-discovery/getDataSchema.test.ts

@@ -168,6 +168,22 @@ t.test("test max depth", async (t) => {
   t.notOk(JSON.stringify(schema2).includes('"type":"string"'));
 });
 
+t.test("array nesting counts toward max depth", async (t) => {
+  const wrapInArrays = (levels: number): unknown => {
+    let nested: unknown = { value: "leaf" };
+    for (let i = 0; i < levels; i++) {
+      nested = [nested];
+    }
+    return nested;
+  };
+
+  const shallow = getDataSchema(wrapInArrays(19));
+  t.ok(JSON.stringify(shallow).includes('"value"'));
+
+  const deep = getDataSchema(wrapInArrays(21));
+  t.notOk(JSON.stringify(deep).includes('"value"'));
+});
+
 t.test("test max properties", async (t) => {
   const generateObjectWithProperties = (count: number) => {
     const obj: any = {};

library/agent/api-discovery/getDataSchema.ts

@@ -55,7 +55,7 @@ export function getDataSchema(data: unknown, depth = 0): DataSchema {
     return {
       type: "array",
       // Assume that the array is homogenous (for performance reasons)
-      items: data.length > 0 ? getDataSchema(data[0]) : null,
+      items: data.length > 0 ? getDataSchema(data[0], depth + 1) : null,
     };
   }
 

library/helpers/attackPath.test.ts

@@ -97,6 +97,34 @@ t.test("first item in array", async (t) => {
   t.same(get("id = 1", ["id = 1"]), [".[0]"]);
 });
 
+t.test("deeply nested arrays do not cause a stack overflow", async (t) => {
+  // Build a deep pure-array structure: [[[...["payload"]...]]]
+  let nested: unknown = ["payload"];
+  for (let i = 0; i < 15000; i++) {
+    nested = [nested];
+  }
+
+  get("payload", nested);
+  // Payload is beyond MAX_DEPTH so it should not be found
+  t.same(get("payload", nested), []);
+});
+
+t.test(
+  "array join on deeply nested array does not cause a stack overflow",
+  async (t) => {
+    let deepNested: unknown = ["x"];
+    for (let i = 0; i < 15000; i++) {
+      deepNested = [deepNested, "y"];
+    }
+
+    // Place the nested array before the attack payload key so join() is called first.
+    const obj = { nested: deepNested, payload: "SELECT 1" };
+
+    get("SELECT 1", obj);
+    t.same(get("SELECT 1", obj), [".payload"]);
+  }
+);
+
 t.test("it checks max matches when iterating over object props", async (t) => {
   const testObj = {
     a: ["test"],

library/helpers/attackPath.ts

@@ -85,21 +85,25 @@ export function getPathsToPayload(
     }
 
     if (Array.isArray(value)) {
-      if (
-        value.length > 1 &&
-        value.length < MAX_ARRAY_LENGTH &&
-        value.join().toLowerCase() === attackPayloadLowercase
-      ) {
-        matches.add(path);
-        return;
+      try {
+        if (
+          value.length > 1 &&
+          value.length < MAX_ARRAY_LENGTH &&
+          value.join().toLowerCase() === attackPayloadLowercase
+        ) {
+          matches.add(path);
+          return;
+        }
+      } catch {
+        // Ignore deeply nested arrays that overflow during native join recursion.
       }
 
       for (const [index, item] of value.entries()) {
         if (matches.found() || index > MAX_ARRAY_LENGTH) {
           break;
         }
 
-        traverse(item, path.concat({ type: "array", index }), depth);
+        traverse(item, path.concat({ type: "array", index }), depth + 1);
       }
     }
 

library/vulnerabilities/nosql-injection/detectNoSQLInjection.overflow.test.ts

@@ -0,0 +1,54 @@
+import * as t from "tap";
+import { detectNoSQLInjection } from "./detectNoSQLInjection";
+import { Context } from "../../agent/Context";
+
+function createContext({
+  query,
+  headers,
+  body,
+  cookies,
+  routeParams,
+}: {
+  query?: Context["query"];
+  body?: Context["body"];
+  headers?: Context["headers"];
+  cookies?: Context["cookies"];
+  routeParams?: Context["routeParams"];
+}): Context {
+  return {
+    remoteAddress: "::1",
+    method: "GET",
+    url: "http://localhost:4000",
+    query: query ? query : {},
+    headers: headers ? headers : {},
+    body: body,
+    cookies: cookies ? cookies : {},
+    routeParams: routeParams ? routeParams : {},
+    source: "express",
+    route: "/posts/:id",
+  };
+}
+
+t.test(
+  "deeply nested array in body does not cause a stack overflow",
+  { timeout: 30 * 1000 },
+  async (t) => {
+    let deepNested: unknown = ["x", "y"];
+    for (let i = 0; i < 4000; i++) {
+      deepNested = [deepNested, "z"];
+    }
+
+    const ctx = createContext({
+      body: { nested: deepNested, username: { $ne: null } },
+    });
+    const filter = { username: { $ne: null } };
+
+    detectNoSQLInjection(ctx, filter);
+    t.same(detectNoSQLInjection(ctx, filter), {
+      injection: true,
+      source: "body",
+      pathsToPayload: [".username"],
+      payload: { $ne: null },
+    });
+  }
+);

library/vulnerabilities/nosql-injection/detectNoSQLInjection.ts

@@ -6,11 +6,19 @@ import { isPlainObject } from "../../helpers/isPlainObject";
 import { tryDecodeAsJWT } from "../../helpers/tryDecodeAsJWT";
 import { detectDbJsInjection } from "../js-injection/detectDbJsInjection";
 
+// Matches the depth limit used by extractStringsFromUserInput
+const MAX_DEPTH = 1024;
+
 function matchFilterPartInUser(
   userInput: unknown,
   filterPart: Record<string, unknown>,
-  pathToPayload: PathPart[] = []
+  pathToPayload: PathPart[] = [],
+  depth = 0
 ): { match: false } | { match: true; pathToPayload: string } {
+  if (depth > MAX_DEPTH) {
+    return { match: false };
+  }
+
   if (typeof userInput === "string") {
     // Check for js injection in $where
     if (detectDbJsInjection(userInput, filterPart)) {
@@ -25,7 +33,8 @@ function matchFilterPartInUser(
       return matchFilterPartInUser(
         jwt.object,
         filterPart,
-        pathToPayload.concat([{ type: "jwt" }])
+        pathToPayload.concat([{ type: "jwt" }]),
+        depth + 1
       );
     }
   }
@@ -40,7 +49,8 @@ function matchFilterPartInUser(
       const match = matchFilterPartInUser(
         userInput[key],
         filterPart,
-        pathToPayload.concat([{ type: "object", key: key }])
+        pathToPayload.concat([{ type: "object", key: key }]),
+        depth + 1
       );
 
       if (match.match) {
@@ -54,15 +64,25 @@ function matchFilterPartInUser(
       const match = matchFilterPartInUser(
         userInput[index],
         filterPart,
-        pathToPayload.concat([{ type: "array", index: index }])
+        pathToPayload.concat([{ type: "array", index: index }]),
+        depth + 1
       );
 
       if (match.match) {
         return match;
       }
     }
 
-    return matchFilterPartInUser(userInput.join(), filterPart, pathToPayload);
+    try {
+      return matchFilterPartInUser(
+        userInput.join(),
+        filterPart,
+        pathToPayload,
+        depth + 1
+      );
+    } catch {
+      // Ignore deeply nested arrays that overflow during native join recursion.
+    }
   }
 
   return {