diff --git a/library/src/agent/Agent.ts b/library/src/agent/Agent.ts index b0f4a0fa1..951b2ef3f 100644 --- a/library/src/agent/Agent.ts +++ b/library/src/agent/Agent.ts @@ -5,8 +5,9 @@ import { getAgentVersion } from "../helpers/getAgentVersion"; import { ip } from "../helpers/ipAddress"; import { filterEmptyRequestHeaders } from "../helpers/filterEmptyRequestHeaders"; import { API } from "./api/API"; -import { AgentInfo, Kind } from "./api/Event"; +import { AgentInfo } from "./api/Event"; import { Token } from "./api/Token"; +import { Kind } from "./Attack"; import { Context } from "./Context"; import { InspectionStatistics } from "./InspectionStatistics"; import { Logger } from "./logger/Logger"; diff --git a/library/src/agent/Attack.ts b/library/src/agent/Attack.ts new file mode 100644 index 000000000..8349408f0 --- /dev/null +++ b/library/src/agent/Attack.ts @@ -0,0 +1,10 @@ +export type Kind = "nosql_injection" | "sql_injection"; + +export function attackKindHumanName(kind: Kind) { + switch (kind) { + case "nosql_injection": + return "NoSQL injection"; + case "sql_injection": + return "SQL injection"; + } +} diff --git a/library/src/agent/InspectionStatistics.test.ts b/library/src/agent/InspectionStatistics.test.ts index 200b9f326..1d49b2170 100644 --- a/library/src/agent/InspectionStatistics.test.ts +++ b/library/src/agent/InspectionStatistics.test.ts @@ -10,15 +10,18 @@ t.test("it keeps track of amount of calls", async () => { stats.onInspectedCall({ module: "mongodb", - withoutContext: false, blocked: false, durationInMs: 0.1, + attackDetected: false, }); t.same(stats.getStats(), { mongodb: { - blocked: 0, - allowed: 1, + attacksDetected: { + total: 0, + blocked: 0, + }, + interceptorThrewError: 0, withoutContext: 0, total: 1, averageInMS: 0.1, @@ -27,19 +30,20 @@ t.test("it keeps track of amount of calls", async () => { 75: 0.1, 90: 0.1, 95: 0.1, + 99: 0.1, }, }, }); - stats.onInspectedCall({ - module: "mongodb", - withoutContext: true, - }); + stats.inspectedCallWithoutContext("mongodb"); t.same(stats.getStats(), { mongodb: { - blocked: 0, - allowed: 2, + attacksDetected: { + total: 0, + blocked: 0, + }, + interceptorThrewError: 0, withoutContext: 1, total: 2, averageInMS: 0.1, @@ -48,52 +52,83 @@ t.test("it keeps track of amount of calls", async () => { 75: 0.1, 90: 0.1, 95: 0.1, + 99: 0.1, + }, + }, + }); + + stats.interceptorThrewError("mongodb"); + + t.same(stats.getStats(), { + mongodb: { + attacksDetected: { + total: 0, + blocked: 0, + }, + interceptorThrewError: 1, + withoutContext: 1, + total: 3, + averageInMS: 0.1, + percentiles: { + 50: 0.1, + 75: 0.1, + 90: 0.1, + 95: 0.1, + 99: 0.1, }, }, }); stats.onInspectedCall({ module: "mongodb", - withoutContext: false, - blocked: true, - durationInMs: 0.5, + blocked: false, + durationInMs: 0.1, + attackDetected: true, }); t.same(stats.getStats(), { mongodb: { - blocked: 1, - allowed: 2, + attacksDetected: { + total: 1, + blocked: 0, + }, + interceptorThrewError: 1, withoutContext: 1, - total: 3, - averageInMS: 0.3, + total: 4, + averageInMS: 0.1, percentiles: { 50: 0.1, - 75: 0.5, - 90: 0.5, - 95: 0.5, + 75: 0.1, + 90: 0.1, + 95: 0.1, + 99: 0.1, }, }, }); stats.onInspectedCall({ module: "mongodb", - withoutContext: false, blocked: true, durationInMs: 0.3, + attackDetected: true, }); t.same(stats.getStats(), { mongodb: { - blocked: 2, - allowed: 2, + attacksDetected: { + total: 2, + blocked: 1, + }, + interceptorThrewError: 1, withoutContext: 1, - total: 4, - averageInMS: 0.3, + total: 5, + averageInMS: 0.16666666666666666, percentiles: { - 50: 0.3, - 75: 0.5, - 90: 0.5, - 95: 0.5, + 50: 0.1, + 75: 0.3, + 90: 0.3, + 95: 0.3, + 99: 0.3, }, }, }); @@ -103,9 +138,9 @@ t.test("it keeps track of amount of calls", async () => { for (let i = 0; i < 50; i++) { stats.onInspectedCall({ module: "mongodb", - withoutContext: false, blocked: false, durationInMs: 0.1, + attackDetected: false, }); } diff --git a/library/src/agent/InspectionStatistics.ts b/library/src/agent/InspectionStatistics.ts index 656e15019..b13392cbb 100644 --- a/library/src/agent/InspectionStatistics.ts +++ b/library/src/agent/InspectionStatistics.ts @@ -1,11 +1,14 @@ import { percentiles } from "../helpers/percentiles"; type ModuleStats = { - blocked: number; - allowed: number; withoutContext: number; total: number; timings: number[]; + interceptorThrewError: number; + attacksDetected: { + total: number; + blocked: number; + }; }; type ModuleStatsEnriched = { @@ -13,18 +16,6 @@ type ModuleStatsEnriched = { percentiles: Record; } & Omit; -type InspectCallArguments = - | { - module: string; - withoutContext: false; - blocked: boolean; - durationInMs: number; - } - | { - module: string; - withoutContext: true; - }; - export class InspectionStatistics { private stats: Record = {}; private readonly maxTimings: number; @@ -51,20 +42,27 @@ export class InspectionStatistics { stats[module] = { total: moduleStats.total, - blocked: moduleStats.blocked, - allowed: moduleStats.allowed, + attacksDetected: { + total: moduleStats.attacksDetected.total, + blocked: moduleStats.attacksDetected.blocked, + }, + interceptorThrewError: moduleStats.interceptorThrewError, withoutContext: moduleStats.withoutContext, averageInMS, percentiles: {}, }; if (timings.length > 0) { - const [p50, p75, p90, p95] = percentiles([50, 75, 90, 99], timings); + const [p50, p75, p90, p95, p99] = percentiles( + [50, 75, 90, 95, 99], + timings + ); stats[module].percentiles = { "50": p50, "75": p75, "90": p90, "95": p95, + "99": p99, }; } } @@ -72,37 +70,58 @@ export class InspectionStatistics { return stats; } - onInspectedCall(args: InspectCallArguments) { - const { module } = args; - + private ensureModuleStats(module: string) { if (!this.stats[module]) { this.stats[module] = { - blocked: 0, - allowed: 0, withoutContext: 0, total: 0, timings: [], + interceptorThrewError: 0, + attacksDetected: { + total: 0, + blocked: 0, + }, }; } + } + inspectedCallWithoutContext(module: string) { + this.ensureModuleStats(module); this.stats[module].total += 1; + this.stats[module].withoutContext += 1; + } - if (args.withoutContext) { - this.stats[module].withoutContext += 1; - this.stats[module].allowed += 1; - return; - } + interceptorThrewError(module: string) { + this.ensureModuleStats(module); + this.stats[module].total += 1; + this.stats[module].interceptorThrewError += 1; + } - this.stats[module].timings.push(args.durationInMs); + onInspectedCall({ + module, + blocked, + attackDetected, + durationInMs, + }: { + module: string; + durationInMs: number; + attackDetected: boolean; + blocked: boolean; + }) { + this.ensureModuleStats(module); + + this.stats[module].total += 1; + this.stats[module].timings.push(durationInMs); if (this.stats[module].timings.length > this.maxTimings) { this.stats[module].timings.shift(); } - if (args.blocked) { - this.stats[module].blocked += 1; - } else { - this.stats[module].allowed += 1; + if (attackDetected) { + this.stats[module].attacksDetected.total += 1; + if (blocked) { + this.stats[module].attacksDetected.blocked += 1; + } } } } diff --git a/library/src/agent/Source.ts b/library/src/agent/Source.ts index 41967afd1..499975dba 100644 --- a/library/src/agent/Source.ts +++ b/library/src/agent/Source.ts @@ -5,7 +5,7 @@ export type Source = "query" | "body" | "headers" | "cookies"; * @param source A source type (either "query", "body", "headers" or "cookies") * @returns A friendly name for each of these types */ -export function friendlyName(source: Source): string { +export function sourceHumanName(source: Source): string { switch (source) { case "query": return "query parameters"; diff --git a/library/src/agent/api/Event.ts b/library/src/agent/api/Event.ts index 5e254cd86..0dcc510c6 100644 --- a/library/src/agent/api/Event.ts +++ b/library/src/agent/api/Event.ts @@ -1,3 +1,4 @@ +import { Kind } from "../Attack"; import { Source } from "../Source"; export type AgentInfo = { @@ -21,8 +22,6 @@ type Started = { time: number; }; -export type Kind = "nosql_injection" | "sql_injection"; - type DetectedAttack = { type: "detected_attack"; request: { @@ -51,8 +50,10 @@ type ModuleName = string; type Stats = Record< ModuleName, { - blocked: number; - allowed: number; + attacksDetected: { + total: number; + blocked: number; + }; withoutContext: number; total: number; averageInMS: number; diff --git a/library/src/agent/applyHooks.test.ts b/library/src/agent/applyHooks.test.ts index 0433144d0..9e82a4928 100644 --- a/library/src/agent/applyHooks.test.ts +++ b/library/src/agent/applyHooks.test.ts @@ -102,12 +102,6 @@ t.test("it adds try/catch around the wrapped method", async (t) => { connection.modifyArguments("execute", () => { throw new Error("THIS SHOULD BE CATCHED"); }); - connection.inspect("ping", () => { - throw new Error("Aikido guard has blocked a SQL injection"); - }); - connection.modifyArguments("rollback", () => { - throw new Error("Aikido guard has blocked a SQL injection"); - }); const { agent, logger } = createAgent(); t.same(applyHooks(hooks, agent), { @@ -142,19 +136,5 @@ t.test("it adds try/catch around the wrapped method", async (t) => { 'Internal error in module "mysql2" in method "execute"', ]); - const error = await t.rejects(() => - runWithContext(context, () => actualConnection.ping()) - ); - if (error instanceof Error) { - t.equal(error.message, "Aikido guard has blocked a SQL injection"); - } - - const error2 = await t.rejects(() => - runWithContext(context, () => actualConnection.rollback()) - ); - if (error2 instanceof Error) { - t.equal(error2.message, "Aikido guard has blocked a SQL injection"); - } - await actualConnection.end(); }); diff --git a/library/src/agent/applyHooks.ts b/library/src/agent/applyHooks.ts index 044568ef5..e52558df5 100644 --- a/library/src/agent/applyHooks.ts +++ b/library/src/agent/applyHooks.ts @@ -1,16 +1,21 @@ /* eslint-disable max-lines-per-function */ import { join } from "node:path"; -import { unwrap, wrap } from "shimmer"; +import { wrap } from "shimmer"; import { getPackageVersion } from "../helpers/getPackageVersion"; import { satisfiesVersion } from "../helpers/satisfiesVersion"; import { Agent } from "./Agent"; +import { attackKindHumanName } from "./Attack"; import { getContext } from "./Context"; import { Hooks } from "./hooks/Hooks"; -import { MethodInterceptor } from "./hooks/MethodInterceptor"; +import { + InterceptorResult, + MethodInterceptor, +} from "./hooks/MethodInterceptor"; import { ModifyingArgumentsMethodInterceptor } from "./hooks/ModifyingArgumentsInterceptor"; import { Package } from "./hooks/Package"; import { WrappableFile } from "./hooks/WrappableFile"; import { WrappableSubject } from "./hooks/WrappableSubject"; +import { sourceHumanName } from "./Source"; /** * Hooks allows you to register packages and then wrap specific methods on @@ -94,10 +99,6 @@ function wrapPackage(pkg: Package, subjects: WrappableSubject[], agent: Agent) { ); } -function isAikidoGuardBlockError(error: Error) { - return error.message.startsWith("Aikido guard"); -} - /** * Wraps a method call with an interceptor that doesn't modify the arguments of the method call. */ @@ -113,10 +114,7 @@ function wrapWithoutArgumentModification( const context = getContext(); if (!context) { - agent.getInspectionStatistics().onInspectedCall({ - module: module, - withoutContext: true, - }); + agent.getInspectionStatistics().inspectedCallWithoutContext(module); return original.apply( // @ts-expect-error We don't now the type of this @@ -129,38 +127,45 @@ function wrapWithoutArgumentModification( // eslint-disable-next-line prefer-rest-params const args = Array.from(arguments); const start = performance.now(); + let result: InterceptorResult = undefined; try { // @ts-expect-error We don't now the type of this - method.getInterceptor()(args, this, agent, context); - const end = performance.now(); - agent.getInspectionStatistics().onInspectedCall({ - module: module, - withoutContext: false, - blocked: false, - durationInMs: end - start, - }); + result = method.getInterceptor()(args, this, agent, context); } catch (error: any) { - const end = performance.now(); - const isAikidoGuardBlock = isAikidoGuardBlockError(error); - agent.getInspectionStatistics().onInspectedCall({ + agent.getInspectionStatistics().interceptorThrewError(module); + agent.onErrorThrownByInterceptor({ + error: error, + method: method.getName(), module: module, - withoutContext: false, - blocked: isAikidoGuardBlock, - durationInMs: end - start, }); + } - // Rethrow our own errors - // Otherwise we cannot block injections - if (isAikidoGuardBlock) { - throw error; - } + const end = performance.now(); + agent.getInspectionStatistics().onInspectedCall({ + module: module, + attackDetected: !!result, + blocked: agent.shouldBlock(), + durationInMs: end - start, + }); - agent.onErrorThrownByInterceptor({ - error: error, - method: method.getName(), + if (result) { + agent.onDetectedAttack({ module: module, + kind: result.kind, + source: result.source, + blocked: agent.shouldBlock(), + stack: new Error().stack!, + path: result.pathToPayload, + metadata: result.metadata, + request: context, }); + + if (agent.shouldBlock()) { + throw new Error( + `Aikido guard has blocked a ${attackKindHumanName(result.kind)}: ${result.operation}(...) originating from ${sourceHumanName(result.source)} (${result.pathToPayload})` + ); + } } return original.apply( @@ -193,12 +198,6 @@ function wrapWithArgumentModification( // @ts-expect-error We don't now the type of this updatedArgs = method.getInterceptor()(args, this, agent); } catch (error: any) { - // Rethrow our own errors - // Otherwise we cannot block injections - if (isAikidoGuardBlockError(error)) { - throw error; - } - agent.onErrorThrownByInterceptor({ error: error, method: method.getName(), diff --git a/library/src/agent/hooks/MethodInterceptor.ts b/library/src/agent/hooks/MethodInterceptor.ts index d831967e6..1faf87fc3 100644 --- a/library/src/agent/hooks/MethodInterceptor.ts +++ b/library/src/agent/hooks/MethodInterceptor.ts @@ -1,12 +1,22 @@ import { Agent } from "../Agent"; +import { Kind } from "../Attack"; import { Context } from "../Context"; +import { Source } from "../Source"; + +export type InterceptorResult = { + operation: string; + kind: Kind; + source: Source; + pathToPayload: string; + metadata: Record; +} | void; export type Interceptor = ( args: unknown[], subject: unknown, agent: Agent, context: Context -) => void; +) => InterceptorResult; export class MethodInterceptor { constructor( diff --git a/library/src/sinks/MongoDB.test.ts b/library/src/sinks/MongoDB.test.ts index 3bf0f84ae..f55086cf8 100644 --- a/library/src/sinks/MongoDB.test.ts +++ b/library/src/sinks/MongoDB.test.ts @@ -104,7 +104,7 @@ t.test("it detects NoSQL injections", async (t) => { }); if (bulkError instanceof Error) { - t.equal( + t.same( bulkError.message, "Aikido guard has blocked a NoSQL injection: MongoDB.Collection.bulkWrite(...) originating from body (.myTitle)" ); @@ -117,7 +117,7 @@ t.test("it detects NoSQL injections", async (t) => { }); if (error instanceof Error) { - t.equal( + t.same( error.message, "Aikido guard has blocked a NoSQL injection: MongoDB.Collection.find(...) originating from body (.myTitle)" ); diff --git a/library/src/sinks/MongoDB.ts b/library/src/sinks/MongoDB.ts index e70c90628..ff6ff3296 100644 --- a/library/src/sinks/MongoDB.ts +++ b/library/src/sinks/MongoDB.ts @@ -1,11 +1,10 @@ /* eslint-disable prefer-rest-params */ import { Collection } from "mongodb"; -import { Agent } from "../agent/Agent"; import { Hooks } from "../agent/hooks/Hooks"; +import { InterceptorResult } from "../agent/hooks/MethodInterceptor"; import { detectNoSQLInjection } from "../vulnerabilities/nosql-injection/detectNoSQLInjection"; import { isPlainObject } from "../helpers/isPlainObject"; import { Context } from "../agent/Context"; -import { friendlyName } from "../agent/Source"; import { Wrapper } from "../agent/Wrapper"; const OPERATIONS_WITH_FILTER = [ @@ -42,71 +41,67 @@ export class MongoDB implements Wrapper { private inspectFilter( db: string, collection: string, - agent: Agent, request: Context, filter: unknown, operation: string - ) { + ): InterceptorResult { const result = detectNoSQLInjection(request, filter); if (result.injection) { - agent.onDetectedAttack({ - module: "mongodb", + return { + operation: `MongoDB.Collection.${operation}`, kind: "nosql_injection", - blocked: agent.shouldBlock(), source: result.source, - request: request, - stack: new Error().stack || "", - path: result.pathToPayload, + pathToPayload: result.pathToPayload, metadata: { db: db, collection: collection, operation: operation, filter: JSON.stringify(filter), }, - }); - - if (agent.shouldBlock()) { - throw new Error( - `Aikido guard has blocked a NoSQL injection: MongoDB.Collection.${operation}(...) originating from ${friendlyName(result.source)} (${result.pathToPayload})` - ); - } + }; } } private inspectBulkWriteOperation( operation: BulkWriteOperation, collection: Collection, - agent: Agent, context: Context - ) { - BULK_WRITE_OPERATIONS_WITH_FILTER.forEach((command) => { - const options = operation[command]; + ): InterceptorResult { + for (const op of BULK_WRITE_OPERATIONS_WITH_FILTER) { + const options = operation[op]; if (options && options.filter) { - this.inspectFilter( + return this.inspectFilter( collection.dbName, collection.collectionName, - agent, context, options.filter, "bulkWrite" ); } - }); + } } private inspectBulkWrite( args: unknown[], collection: Collection, - agent: Agent, context: Context ) { if (Array.isArray(args[0]) && args[0].length > 0) { const operations: BulkWriteOperation[] = args[0]; - operations.forEach((operation) => { - this.inspectBulkWriteOperation(operation, collection, agent, context); - }); + + for (const operation of operations) { + const result = this.inspectBulkWriteOperation( + operation, + collection, + context + ); + + if (result) { + return result; + } + } } } @@ -114,15 +109,14 @@ export class MongoDB implements Wrapper { operation: string, args: unknown[], collection: Collection, - agent: Agent, context: Context - ): void { + ): InterceptorResult { if (args.length > 0 && isPlainObject(args[0])) { const filter = args[0]; - this.inspectFilter( + + return this.inspectFilter( collection.dbName, collection.collectionName, - agent, context, filter, operation @@ -145,14 +139,13 @@ export class MongoDB implements Wrapper { operation, args, collection as Collection, - agent, context ) ); }); collection.inspect("bulkWrite", (args, collection, agent, context) => - this.inspectBulkWrite(args, collection as Collection, agent, context) + this.inspectBulkWrite(args, collection as Collection, context) ); } } diff --git a/library/src/sinks/MySQL.test.ts b/library/src/sinks/MySQL.test.ts index a81484233..9df45c30e 100644 --- a/library/src/sinks/MySQL.test.ts +++ b/library/src/sinks/MySQL.test.ts @@ -62,16 +62,16 @@ t.test("it detects SQL injections", async () => { await query("TRUNCATE cats", connection); t.same(await query("SELECT petname FROM `cats`;", connection), []); - const bulkError = await t.rejects(async () => { + const error = await t.rejects(async () => { await runWithContext(context, () => { return connection.query("-- should be blocked"); }); }); - if (bulkError instanceof Error) { - t.equal( - bulkError.message, - "Aikido guard has blocked a SQL injection: -- should be blocked originating from body" + if (error instanceof Error) { + t.same( + error.message, + "Aikido guard has blocked a SQL injection: MySQL.query(...) originating from body (UNKOWN)" ); } diff --git a/library/src/sinks/MySQL.ts b/library/src/sinks/MySQL.ts index d27181cb7..5d7b690e7 100644 --- a/library/src/sinks/MySQL.ts +++ b/library/src/sinks/MySQL.ts @@ -1,14 +1,24 @@ import { Agent } from "../agent/Agent"; import { Context } from "../agent/Context"; import { Hooks } from "../agent/hooks/Hooks"; +import { InterceptorResult } from "../agent/hooks/MethodInterceptor"; import { Wrapper } from "../agent/Wrapper"; import { checkContextForSqlInjection } from "../vulnerabilities/sql-injection/detectSQLInjection"; export class MySQL implements Wrapper { - private inspectQuery(args: unknown[], agent: Agent, context: Context) { + private inspectQuery( + args: unknown[], + agent: Agent, + context: Context + ): InterceptorResult { if (args.length > 0 && typeof args[0] === "string" && args[0].length > 0) { const sql = args[0]; - checkContextForSqlInjection(sql, context, agent, "mysql"); + + return checkContextForSqlInjection({ + sql: sql, + context: context, + operation: "MySQL.query", + }); } } diff --git a/library/src/sinks/MySQL2.test.ts b/library/src/sinks/MySQL2.test.ts index 4a5ff9724..da285fa1c 100644 --- a/library/src/sinks/MySQL2.test.ts +++ b/library/src/sinks/MySQL2.test.ts @@ -60,16 +60,16 @@ t.test("it detects SQL injections", async () => { const [rows] = await connection.query("SELECT petname FROM `cats`;"); t.same(rows, []); - const bulkError = await t.rejects(async () => { + const error = await t.rejects(async () => { await runWithContext(dangerousContext, () => { return connection.query("-- should be blocked"); }); }); - if (bulkError instanceof Error) { - t.equal( - bulkError.message, - "Aikido guard has blocked a SQL injection: -- should be blocked originating from body" + if (error instanceof Error) { + t.same( + error.message, + "Aikido guard has blocked a SQL injection: mysql2.query(...) originating from body (UNKOWN)" ); } diff --git a/library/src/sinks/MySQL2.ts b/library/src/sinks/MySQL2.ts index a02f84f92..e1c4fa23f 100644 --- a/library/src/sinks/MySQL2.ts +++ b/library/src/sinks/MySQL2.ts @@ -1,14 +1,24 @@ import { Agent } from "../agent/Agent"; import { Context } from "../agent/Context"; import { Hooks } from "../agent/hooks/Hooks"; +import { InterceptorResult } from "../agent/hooks/MethodInterceptor"; import { Wrapper } from "../agent/Wrapper"; import { checkContextForSqlInjection } from "../vulnerabilities/sql-injection/detectSQLInjection"; export class MySQL2 implements Wrapper { - private inspectQuery(args: unknown[], agent: Agent, context: Context) { + private inspectQuery( + operation: string, + args: unknown[], + context: Context + ): InterceptorResult { if (args.length > 0 && typeof args[0] === "string" && args[0].length > 0) { const sql = args[0]; - checkContextForSqlInjection(sql, context, agent, "mysql2"); + + return checkContextForSqlInjection({ + operation: operation, + sql: sql, + context: context, + }); } } @@ -19,11 +29,11 @@ export class MySQL2 implements Wrapper { ); connection.inspect("query", (args, subject, agent, context) => - this.inspectQuery(args, agent, context) + this.inspectQuery("mysql2.query", args, context) ); connection.inspect("execute", (args, subject, agent, context) => - this.inspectQuery(args, agent, context) + this.inspectQuery("mysql2.execute", args, context) ); } } diff --git a/library/src/sinks/Postgres.pool.test.ts b/library/src/sinks/Postgres.pool.test.ts index b7397361c..829b6116d 100644 --- a/library/src/sinks/Postgres.pool.test.ts +++ b/library/src/sinks/Postgres.pool.test.ts @@ -47,15 +47,15 @@ t.test("it detects SQL injections", async () => { await client.query("TRUNCATE cats"); t.same((await client.query("SELECT petname FROM cats;")).rows, []); - const bulkError = await t.rejects(async () => { + const error = await t.rejects(async () => { await runWithContext(context, () => { return client.query("-- should be blocked"); }); }); - if (bulkError instanceof Error) { - t.equal( - bulkError.message, - "Aikido guard has blocked a SQL injection: -- should be blocked originating from body" + if (error instanceof Error) { + t.same( + error.message, + "Aikido guard has blocked a SQL injection: pg.query(...) originating from body (UNKOWN)" ); } @@ -65,7 +65,7 @@ t.test("it detects SQL injections", async () => { }); }); if (undefinedQueryError instanceof Error) { - t.equal( + t.same( undefinedQueryError.message, "Client was passed a null or undefined query" ); diff --git a/library/src/sinks/Postgres.test.ts b/library/src/sinks/Postgres.test.ts index d8c9c1b1e..39e68b53d 100644 --- a/library/src/sinks/Postgres.test.ts +++ b/library/src/sinks/Postgres.test.ts @@ -46,15 +46,15 @@ t.test("it detects SQL injections", async () => { await client.query("TRUNCATE cats"); t.same((await client.query("SELECT petname FROM cats;")).rows, []); - const bulkError = await t.rejects(async () => { + const error = await t.rejects(async () => { await runWithContext(context, () => { return client.query("-- should be blocked"); }); }); - if (bulkError instanceof Error) { - t.equal( - bulkError.message, - "Aikido guard has blocked a SQL injection: -- should be blocked originating from body" + if (error instanceof Error) { + t.same( + error.message, + "Aikido guard has blocked a SQL injection: pg.query(...) originating from body (UNKOWN)" ); } @@ -64,7 +64,7 @@ t.test("it detects SQL injections", async () => { }); }); if (undefinedQueryError instanceof Error) { - t.equal( + t.same( undefinedQueryError.message, "Client was passed a null or undefined query" ); diff --git a/library/src/sinks/Postgres.ts b/library/src/sinks/Postgres.ts index e73385a20..79b71f5f2 100644 --- a/library/src/sinks/Postgres.ts +++ b/library/src/sinks/Postgres.ts @@ -1,14 +1,20 @@ import { Agent } from "../agent/Agent"; import { Hooks } from "../agent/hooks/Hooks"; +import { InterceptorResult } from "../agent/hooks/MethodInterceptor"; import { Wrapper } from "../agent/Wrapper"; import { Context } from "../agent/Context"; import { checkContextForSqlInjection } from "../vulnerabilities/sql-injection/detectSQLInjection"; export class Postgres implements Wrapper { - private inspectQuery(args: unknown[], agent: Agent, context: Context) { + private inspectQuery(args: unknown[], context: Context): InterceptorResult { if (args.length > 0 && typeof args[0] === "string" && args[0].length > 0) { const sql: string = args[0]; - checkContextForSqlInjection(sql, context, agent, "postgres"); + + return checkContextForSqlInjection({ + sql: sql, + context: context, + operation: "pg.query", + }); } } @@ -17,7 +23,7 @@ export class Postgres implements Wrapper { const client = pg.addSubject((exports) => exports.Client.prototype); client.inspect("query", (args, subject, agent, context) => - this.inspectQuery(args, agent, context) + this.inspectQuery(args, context) ); } } diff --git a/library/src/vulnerabilities/sql-injection/detectSQLInjection.ts b/library/src/vulnerabilities/sql-injection/detectSQLInjection.ts index 5a035cc60..cb8c5e8c4 100644 --- a/library/src/vulnerabilities/sql-injection/detectSQLInjection.ts +++ b/library/src/vulnerabilities/sql-injection/detectSQLInjection.ts @@ -1,6 +1,7 @@ import { Agent } from "../../agent/Agent"; import { Context } from "../../agent/Context"; -import { friendlyName, Source } from "../../agent/Source"; +import { InterceptorResult } from "../../agent/hooks/MethodInterceptor"; +import { sourceHumanName, Source } from "../../agent/Source"; import { extractStringsFromUserInput } from "../../helpers/extractStringsFromUserInput"; import { SQL_STRING_CHARS } from "./config"; import { dangerousCharsInInput } from "./dangerousCharsInInput"; @@ -88,40 +89,29 @@ export function userInputOccurrencesSafelyEncapsulated( /** * This function goes over all the different input types in the context and checks - * if it's a possible SQL Injection, if so the function messages an Agent and if - * needed, throws an error to block any further code. - * @param sql The SQL statement that was executed - * @param request The request that might contain a SQL Injection - * @param agent The agent which needs to get a report in case of detection - * @param module The name of the module e.g. postgres, mysql, mssql + * if it's a possible SQL Injection, if so the function returns an InterceptorResult */ -export function checkContextForSqlInjection( - sql: string, - request: Context, - agent: Agent, - module: string -) { +export function checkContextForSqlInjection({ + sql, + operation, + context, +}: { + sql: string; + operation: string; + context: Context; +}): InterceptorResult { for (const source of ["body", "query", "headers", "cookies"] as Source[]) { - if (request[source]) { - const userInput = extractStringsFromUserInput(request[source]); + if (context[source]) { + const userInput = extractStringsFromUserInput(context[source]); for (const str of userInput) { if (detectSQLInjection(sql, str)) { - agent.onDetectedAttack({ - module, + return { + operation: operation, kind: "sql_injection", - blocked: agent.shouldBlock(), - source, - request: request, - stack: new Error().stack || "", - path: "UNKOWN", + source: source, + pathToPayload: "UNKOWN", metadata: {}, - }); - - if (agent.shouldBlock()) { - throw new Error( - `Aikido guard has blocked a SQL injection: ${str} originating from ${friendlyName(source)}` - ); - } + }; } } }