diff --git a/library/src/integrations/MongoDB.ts b/library/src/integrations/MongoDB.ts index 2651d654b..8b34a16a4 100644 --- a/library/src/integrations/MongoDB.ts +++ b/library/src/integrations/MongoDB.ts @@ -68,7 +68,7 @@ export class MongoDB implements Integration { source: result.source, request: request, stack: new Error().stack || "", - path: result.path, + path: result.pathToPayload, metadata: { db: db, collection: collection, @@ -79,7 +79,7 @@ export class MongoDB implements Integration { if (agent.shouldBlock()) { throw new Error( - `Aikido guard has blocked a NoSQL injection: MongoDB.Collection.${operation}(...) originating from ${friendlyName(result.source)} (${result.path})` + `Aikido guard has blocked a NoSQL injection: MongoDB.Collection.${operation}(...) originating from ${friendlyName(result.source)} (${result.pathToPayload})` ); } } diff --git a/library/src/vulnerabilities/detectNoSQLInjection.test.ts b/library/src/vulnerabilities/detectNoSQLInjection.test.ts index db1bff0ca..4d6c78172 100644 --- a/library/src/vulnerabilities/detectNoSQLInjection.test.ts +++ b/library/src/vulnerabilities/detectNoSQLInjection.test.ts @@ -94,7 +94,7 @@ t.test("using $ne in query parameter", async (t) => { title: { $ne: null }, } ), - { injection: true, source: "query", path: ".title" } + { injection: true, source: "query", pathToPayload: ".title" } ); }); @@ -122,7 +122,7 @@ t.test("using $ne in body", async (t) => { title: { $ne: null }, } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -136,7 +136,7 @@ t.test("using $ne in body (different name)", async (t) => { myTitle: { $ne: null }, } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -150,7 +150,7 @@ t.test("using $ne in headers with different name", async (t) => { someField: { $ne: null }, } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -171,7 +171,7 @@ t.test("using $ne inside $and", async (t) => { ], } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -192,7 +192,7 @@ t.test("using $ne inside $or", async (t) => { ], } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -213,7 +213,7 @@ t.test("using $ne inside $nor", async (t) => { ], } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -229,7 +229,7 @@ t.test("using $ne inside $not", async (t) => { }, } ), - { injection: true, source: "body", path: ".title" } + { injection: true, source: "body", pathToPayload: ".title" } ); }); @@ -248,7 +248,7 @@ t.test("using $ne nested in body", async (t) => { { injection: true, source: "body", - path: ".nested.nested", + pathToPayload: ".nested.nested", } ); }); @@ -279,7 +279,7 @@ t.test("using $ne in JWT in headers", async (t) => { { injection: true, source: "headers", - path: ".Authorization.username", + pathToPayload: ".Authorization.username", } ); }); @@ -310,7 +310,7 @@ t.test("using $ne in JWT in bearer header", async (t) => { { injection: true, source: "headers", - path: ".Authorization.username", + pathToPayload: ".Authorization.username", } ); }); @@ -341,7 +341,7 @@ t.test("using $ne in JWT in cookies", async (t) => { { injection: true, source: "cookies", - path: ".session.username", + pathToPayload: ".session.username", } ); }); @@ -375,7 +375,7 @@ t.test("using $gt in query parameter", async (t) => { age: { $gt: "21" }, } ), - { injection: true, source: "query", path: ".age" } + { injection: true, source: "query", pathToPayload: ".age" } ); }); @@ -389,7 +389,7 @@ t.test("using $gt and $lt in query parameter", async (t) => { age: { $gt: "21", $lt: "100" }, } ), - { injection: true, source: "body", path: ".age" } + { injection: true, source: "body", pathToPayload: ".age" } ); }); @@ -403,7 +403,7 @@ t.test("using $gt and $lt in query parameter (different name)", async (t) => { myAge: { $gt: "21", $lt: "100" }, } ), - { injection: true, source: "body", path: ".age" } + { injection: true, source: "body", pathToPayload: ".age" } ); }); @@ -425,7 +425,7 @@ t.test("using $gt and $lt in query parameter (nested)", async (t) => { ], } ), - { injection: true, source: "body", path: ".nested.nested.age" } + { injection: true, source: "body", pathToPayload: ".nested.nested.age" } ); }); @@ -449,7 +449,7 @@ t.test("using $gt and $lt in query parameter (root)", async (t) => { ], } ), - { injection: true, source: "body", path: "." } + { injection: true, source: "body", pathToPayload: "." } ); }); @@ -473,7 +473,7 @@ t.test("$where", async (t) => { ], } ), - { injection: true, source: "body", path: "." } + { injection: true, source: "body", pathToPayload: "." } ); }); @@ -495,7 +495,7 @@ t.test("array body", async (t) => { ], } ), - { injection: true, source: "body", path: ".[0]" } + { injection: true, source: "body", pathToPayload: ".[0]" } ); }); diff --git a/library/src/vulnerabilities/detectNoSQLInjection.ts b/library/src/vulnerabilities/detectNoSQLInjection.ts index 3978c4632..80092bc71 100644 --- a/library/src/vulnerabilities/detectNoSQLInjection.ts +++ b/library/src/vulnerabilities/detectNoSQLInjection.ts @@ -4,57 +4,87 @@ import { tryDecodeAsJWT } from "../helpers/jwt"; import { Context } from "../agent/Context"; import { Source } from "../agent/Source"; -type DetectionResult = - | { injection: true; source: Source; path: string } - | { injection: false }; +type PathPart = + | { type: "jwt" } + | { type: "object"; key: string } + | { type: "array"; index: number }; + +function buildPathToPayload(pathToPayload: PathPart[]): string { + if (pathToPayload.length === 0) { + return "."; + } + + return pathToPayload.reduce((acc, part) => { + if (part.type === "jwt") { + return `${acc}`; + } + + if (part.type === "object") { + return `${acc}.${part.key}`; + } + + if (part.type === "array") { + return `${acc}.[${part.index}]`; + } + + return acc; + }, ""); +} function matchFilterPartInUser( - user: unknown, + userInput: unknown, filterPart: Record, - path = "" -): string | null { - if (typeof user === "string") { - const jwt = tryDecodeAsJWT(user); + pathToPayload: PathPart[] = [] +): { match: false } | { match: true; pathToPayload: string } { + if (typeof userInput === "string") { + const jwt = tryDecodeAsJWT(userInput); if (jwt.jwt) { - return matchFilterPartInUser(jwt.object, filterPart, `${path}`); + return matchFilterPartInUser( + jwt.object, + filterPart, + pathToPayload.concat([{ type: "jwt" }]) + ); } } - if (isDeepStrictEqual(user, filterPart)) { - return path; + if (isDeepStrictEqual(userInput, filterPart)) { + return { match: true, pathToPayload: buildPathToPayload(pathToPayload) }; } - if (isPlainObject(user)) { - for (const key in user) { + if (isPlainObject(userInput)) { + for (const key in userInput) { const match = matchFilterPartInUser( - user[key], + userInput[key], filterPart, - `${path}.${key}` + pathToPayload.concat([{ type: "object", key: key }]) ); - if (match) { + if (match.match) { return match; } } } - if (Array.isArray(user)) { - for (let index = 0; index < user.length; index++) { + if (Array.isArray(userInput)) { + for (let index = 0; index < userInput.length; index++) { const match = matchFilterPartInUser( - user[index], + userInput[index], filterPart, - `${path}.[${index}]` + pathToPayload.concat([{ type: "array", index: index }]) ); - if (match) { + + if (match.match) { return match; } } } - return null; + return { + match: false, + }; } -function getObjectWithOperators( +function removeKeysThatDontStartWithDollarSign( filter: Record ): Record { return Object.keys(filter).reduce((acc, key) => { @@ -67,38 +97,45 @@ function getObjectWithOperators( } function findFilterPartWithOperators( - user: unknown, + userInput: unknown, partOfFilter: unknown -): string | null { +): { found: false } | { found: true; pathToPayload: string } { if (isPlainObject(partOfFilter)) { - const object = getObjectWithOperators(partOfFilter); + const object = removeKeysThatDontStartWithDollarSign(partOfFilter); if (Object.keys(object).length > 0) { - const path = matchFilterPartInUser(user, object); - if (path) { - return path; + const result = matchFilterPartInUser(userInput, object); + + if (result.match) { + return { found: true, pathToPayload: result.pathToPayload }; } } for (const key in partOfFilter) { - const path = findFilterPartWithOperators(user, partOfFilter[key]); - if (path) { - return path; + const result = findFilterPartWithOperators(userInput, partOfFilter[key]); + + if (result.found) { + return { found: true, pathToPayload: result.pathToPayload }; } } } if (Array.isArray(partOfFilter)) { for (const value of partOfFilter) { - const path = findFilterPartWithOperators(user, value); - if (path) { - return path; + const result = findFilterPartWithOperators(userInput, value); + + if (result.found) { + return { found: true, pathToPayload: result.pathToPayload }; } } } - return null; + return { found: false }; } +type DetectionResult = + | { injection: true; source: Source; pathToPayload: string } + | { injection: false }; + export function detectNoSQLInjection( request: Context, filter: unknown @@ -108,28 +145,14 @@ export function detectNoSQLInjection( } for (const source of ["body", "query", "headers", "cookies"] as Source[]) { - if (source === "body" && isPlainObject(request[source])) { - const object = getObjectWithOperators(filter); - if ( - Object.keys(object).length > 0 && - isDeepStrictEqual(request[source], object) - ) { - return { - injection: true, - source: source, - path: ".", - }; - } - } - if (request[source]) { - const path = findFilterPartWithOperators(request[source], filter); + const result = findFilterPartWithOperators(request[source], filter); - if (path) { + if (result.found) { return { injection: true, source: source, - path: path, + pathToPayload: result.pathToPayload, }; } }