diff --git a/end2end/tests-new/react-router-pg.test.mjs b/end2end/tests-new/react-router-pg.test.mjs new file mode 100644 index 000000000..7b597871c --- /dev/null +++ b/end2end/tests-new/react-router-pg.test.mjs @@ -0,0 +1,152 @@ +import { getRandomPort } from "./utils/get-port.mjs"; +import { spawnSync, spawn } from "node:child_process"; +import { resolve } from "node:path"; +import { timeout } from "./utils/timeout.mjs"; +import { test, before } from "node:test"; +import { equal, fail, match, doesNotMatch } from "node:assert"; + +const pathToAppDir = resolve( + import.meta.dirname, + "../../sample-apps/react-router-pg" +); + +const port = await getRandomPort(); +const port2 = await getRandomPort(); + +before(() => { + const { stderr, status } = spawnSync("npm", ["run", "build"], { + cwd: pathToAppDir, + }); + + if (status !== 0) { + throw new Error(`Failed to build: ${stderr.toString()}`); + } +}); + +test("it blocks request in blocking mode", async () => { + const server = spawn( + `node_modules/.bin/react-router-serve`, + ["./build/server/index.js"], + { + cwd: pathToAppDir, + env: { + ...process.env, + AIKIDO_DEBUG: "true", + AIKIDO_BLOCK: "true", + NODE_OPTIONS: "-r @aikidosec/firewall/instrument", + PORT: port, + }, + } + ); + + try { + server.on("error", (err) => { + fail(err); + }); + + let stdout = ""; + server.stdout.on("data", (data) => { + stdout += data.toString(); + }); + + let stderr = ""; + server.stderr.on("data", (data) => { + stderr += data.toString(); + }); + + // Wait for the server to start + await timeout(2000); + + const formData1 = new FormData(); + formData1.append("catname", "Kitty'); DELETE FROM cats_5;-- H"); + + const formData2 = new FormData(); + formData2.append("catname", "Miau"); + + const [sqlInjection, normalAdd] = await Promise.all([ + fetch(`http://127.0.0.1:${port}/add-cat`, { + method: "POST", + body: formData1, + redirect: "manual", + signal: AbortSignal.timeout(5000), + }), + fetch(`http://127.0.0.1:${port}/add-cat`, { + method: "POST", + body: formData2, + redirect: "manual", + signal: AbortSignal.timeout(5000), + }), + ]); + + equal(sqlInjection.status, 500); + equal(normalAdd.status, 302); // Redirect after successful add + match(stdout, /Starting agent/); + match(stderr, /Zen has blocked an SQL injection/); + } finally { + server.kill(); + } +}); + +test("it does not block request in monitoring mode", async () => { + const server = spawn( + `node_modules/.bin/react-router-serve`, + ["./build/server/index.js"], + { + cwd: pathToAppDir, + env: { + ...process.env, + AIKIDO_DEBUG: "true", + AIKIDO_BLOCK: "false", + NODE_OPTIONS: "-r @aikidosec/firewall/instrument", + PORT: port2, + }, + } + ); + + try { + server.on("error", (err) => { + fail(err); + }); + + let stdout = ""; + server.stdout.on("data", (data) => { + stdout += data.toString(); + }); + + let stderr = ""; + server.stderr.on("data", (data) => { + stderr += data.toString(); + }); + + // Wait for the server to start + await timeout(2000); + + const formData1 = new FormData(); + formData1.append("catname", "Kitty'); DELETE FROM cats_5;-- H"); + + const formData2 = new FormData(); + formData2.append("catname", "Miau"); + + const [sqlInjection, normalAdd] = await Promise.all([ + fetch(`http://127.0.0.1:${port2}/add-cat`, { + method: "POST", + body: formData1, + redirect: "manual", + signal: AbortSignal.timeout(5000), + }), + fetch(`http://127.0.0.1:${port2}/add-cat`, { + method: "POST", + body: formData2, + redirect: "manual", + signal: AbortSignal.timeout(5000), + }), + ]); + + equal(sqlInjection.status, 302); // Redirect even with SQL injection + equal(normalAdd.status, 302); + match(stdout, /Starting agent/); + doesNotMatch(stderr, /Zen has blocked an SQL injection/); + } finally { + server.kill(); + } +}); diff --git a/library/agent/hooks/VersionedPackage.ts b/library/agent/hooks/VersionedPackage.ts index 354d6d505..8fa8ca158 100644 --- a/library/agent/hooks/VersionedPackage.ts +++ b/library/agent/hooks/VersionedPackage.ts @@ -65,6 +65,12 @@ export class VersionedPackage { * The path is relative to the package root. */ addFileInstrumentation(instruction: PackageFileInstrumentationInstruction) { + if (instruction.path instanceof RegExp) { + // Just accept RegExp paths as-is + this.fileInstrumentationInstructions.push(instruction); + return this; + } + if (instruction.path.length === 0) { throw new Error("Path must not be empty"); } diff --git a/library/agent/hooks/instrumentation/codeTransformation.ts b/library/agent/hooks/instrumentation/codeTransformation.ts index fac999ebf..b110177cf 100644 --- a/library/agent/hooks/instrumentation/codeTransformation.ts +++ b/library/agent/hooks/instrumentation/codeTransformation.ts @@ -6,6 +6,13 @@ import { join } from "path"; import { isNewInstrumentationUnitTest } from "../../../helpers/isNewInstrumentationUnitTest"; import { isEsmUnitTest } from "../../../helpers/isEsmUnitTest"; +// path can also be a RegExp, this results in an empty object when serialized to JSON +// serde will try to parse it and crash, so we need to set it to the actual file path +type PackageFileInstrumentationInstructionWASM = Omit< + PackageFileInstrumentationInstructionJSON, + "path" +> & { path: string }; + export function transformCode( pkgName: string, pkgVersion: string, @@ -14,12 +21,17 @@ export function transformCode( pkgLoadFormat: PackageLoadFormat, fileInstructions: PackageFileInstrumentationInstructionJSON ): string { + let wasmInstructions: PackageFileInstrumentationInstructionWASM = { + ...fileInstructions, + path: path, + }; + try { const result = wasm_transform_code_str( pkgName, pkgVersion, code, - JSON.stringify(fileInstructions), + JSON.stringify(wasmInstructions), getSourceType(path, pkgLoadFormat) ); diff --git a/library/agent/hooks/instrumentation/instructions.ts b/library/agent/hooks/instrumentation/instructions.ts index f93347174..c9aa8a3aa 100644 --- a/library/agent/hooks/instrumentation/instructions.ts +++ b/library/agent/hooks/instrumentation/instructions.ts @@ -44,7 +44,7 @@ export function setPackagesToInstrument(_packages: Package[]) { return versionedPackage .getFileInstrumentationInstructions() .map((file) => { - const fileIdentifier = `${pkg.getName()}.${file.path}.${versionedPackage.getRange()}`; + const fileIdentifier = `${pkg.getName()}.${getIdentifier(file.path)}.${versionedPackage.getRange()}`; if (file.accessLocalVariables?.cb) { fileCallbackInfo.set(fileIdentifier, { pkgName: pkg.getName(), @@ -57,7 +57,7 @@ export function setPackagesToInstrument(_packages: Package[]) { versionRange: versionedPackage.getRange(), identifier: fileIdentifier, functions: file.functions.map((func) => { - const identifier = `${pkg.getName()}.${file.path}.${func.name}.${func.nodeType}.${versionedPackage.getRange()}`; + const identifier = `${pkg.getName()}.${getIdentifier(file.path)}.${func.name}.${func.nodeType}.${versionedPackage.getRange()}`; // If bindContext is set to true, but no modifyArgs is defined, modifyArgs will be set to a stub function // The reason for this is that the bindContext logic needs to modify the arguments @@ -117,6 +117,25 @@ export function shouldPatchPackage(name: string): boolean { return packages.has(name); } +function matchesPath( + pathOrPattern: string | RegExp, + filePath: string +): boolean { + if (typeof pathOrPattern === "string") { + return pathOrPattern === filePath; + } + + return pathOrPattern.test(filePath); +} + +function getIdentifier(pathOrPattern: string | RegExp) { + if (typeof pathOrPattern === "string") { + return pathOrPattern; + } + + return pathOrPattern.toString(); +} + export function getPackageFileInstrumentationInstructions( packageName: string, version: string, @@ -128,7 +147,8 @@ export function getPackageFileInstrumentationInstructions( } return instructions.find( - (f) => f.path === filePath && satisfiesVersion(f.versionRange, version) + (f) => + matchesPath(f.path, filePath) && satisfiesVersion(f.versionRange, version) ); } @@ -141,7 +161,7 @@ export function shouldPatchFile( return false; } - return instructions.some((f) => f.path === filePath); + return instructions.some((f) => matchesPath(f.path, filePath)); } export function getFunctionCallbackInfo( diff --git a/library/agent/hooks/instrumentation/types.ts b/library/agent/hooks/instrumentation/types.ts index 049063e8c..31e6c7aa2 100644 --- a/library/agent/hooks/instrumentation/types.ts +++ b/library/agent/hooks/instrumentation/types.ts @@ -119,7 +119,9 @@ export type PackageFunctionInstrumentationInstruction = { }; export type PackageFileInstrumentationInstruction = { - path: string; // Relative path to required file inside the package folder + // Relative path to required file inside the package folder + // You can use a regex in cases where the filename is not known in advance (e.g. chunks generated by a bundler) + path: string | RegExp; functions: PackageFunctionInstrumentationInstruction[]; /** * Access module local variables @@ -132,7 +134,9 @@ export type PackageFileInstrumentationInstruction = { }; export type PackageFileInstrumentationInstructionJSON = { - path: string; // Relative path to required file inside the package folder + // Relative path to required file inside the package folder + // You can use a regex in cases where the filename is not known in advance (e.g. chunks generated by a bundler) + path: string | RegExp; versionRange: string; identifier: string; accessLocalVariables: string[]; diff --git a/library/agent/protect.ts b/library/agent/protect.ts index aa72c3ff2..ccad60ce5 100644 --- a/library/agent/protect.ts +++ b/library/agent/protect.ts @@ -47,6 +47,7 @@ import { Postgresjs } from "../sinks/Postgresjs"; import { Fastify } from "../sources/Fastify"; import { Koa } from "../sources/Koa"; import { Restify } from "../sources/Restify"; +import { ReactRouter } from "../sources/ReactRouter"; import { ClickHouse } from "../sinks/ClickHouse"; import { Prisma } from "../sinks/Prisma"; import { AwsSDKVersion2 } from "../sinks/AwsSDKVersion2"; @@ -165,6 +166,7 @@ export function getWrappers() { new Fastify(), new Koa(), new Restify(), + new ReactRouter(), new ClickHouse(), new Prisma(), new AwsSDKVersion3(), diff --git a/library/helpers/formDataToPlainObject.test.ts b/library/helpers/formDataToPlainObject.test.ts new file mode 100644 index 000000000..f078e6844 --- /dev/null +++ b/library/helpers/formDataToPlainObject.test.ts @@ -0,0 +1,69 @@ +import * as t from "tap"; +import { formDataToPlainObject } from "./formDataToPlainObject"; + +t.test( + "simple", + { + skip: !globalThis.FormData + ? "This Node.js version does not support FormData yet" + : false, + }, + async (t) => { + const formData = new FormData(); + formData.append("abc", "123"); + formData.append("another", "42"); + formData.append("hello", "world"); + + t.same(formDataToPlainObject(formData), { + abc: "123", + another: "42", + hello: "world", + }); + } +); + +t.test( + "with arrays", + { + skip: !globalThis.FormData + ? "This Node.js version does not support FormData yet" + : false, + }, + async (t) => { + const formData = new FormData(); + formData.append("abc", "123"); + formData.append("arr", "1"); + formData.append("arr", "2"); + formData.append("arr", "3"); + + t.same(formDataToPlainObject(formData), { + abc: "123", + arr: ["1", "2", "3"], + }); + } +); + +t.test( + "binary data", + { + skip: + !globalThis.FormData || !globalThis.File + ? "This Node.js version does not support FormData or File yet" + : false, + }, + async (t) => { + const formData = new FormData(); + formData.append("abc", "123"); + formData.append("arr", "2"); + formData.append("arr", "3"); + formData.append( + "file", + new File(["hello"], "hello.txt", { type: "text/plain" }) + ); + + t.same(formDataToPlainObject(formData), { + abc: "123", + arr: ["2", "3"], + }); + } +); diff --git a/library/helpers/formDataToPlainObject.ts b/library/helpers/formDataToPlainObject.ts new file mode 100644 index 000000000..f043568df --- /dev/null +++ b/library/helpers/formDataToPlainObject.ts @@ -0,0 +1,27 @@ +export function formDataToPlainObject(formData: FormData) { + const object: Map = new Map(); + formData.forEach((value, key) => { + if (typeof value !== "string") { + return; + } + + if (object.has(key)) { + // If the key already exists, treat it as an array + const entry = object.get(key); + + if (Array.isArray(entry)) { + // If it's already an array, just push the new value + entry.push(value); + return; + } + + // Convert it to an array + object.set(key, [object.get(key), value]); + return; + } + + object.set(key, value); + }); + + return Object.fromEntries(object); +} diff --git a/library/sources/ReactRouter.ts b/library/sources/ReactRouter.ts new file mode 100644 index 000000000..fb343f42c --- /dev/null +++ b/library/sources/ReactRouter.ts @@ -0,0 +1,31 @@ +import { Hooks } from "../agent/hooks/Hooks"; +import { Wrapper } from "../agent/Wrapper"; +import { wrapRequestBodyParsing } from "./react-router/wrapRequestBodyParsing"; + +export class ReactRouter implements Wrapper { + wrap(hooks: Hooks) { + hooks + .addPackage("react-router") + .withVersion("^7.0.0") + .addFileInstrumentation({ + path: /^dist\/(production|development)\/chunk-[A-Z0-9]+\.mjs$/, + functions: [ + { + // We cannot patch the `Request` global (as Request is also used by fetch calls) + // We're interested in the Request object that gets passed to the server actions + // See https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/server-runtime/data.ts#L26 + nodeType: "FunctionDeclaration", + name: "stripRoutesParam", + operationKind: undefined, + modifyReturnValue: (_, returnValue) => { + if (returnValue instanceof Request) { + wrapRequestBodyParsing(returnValue); + } + + return returnValue; + }, + }, + ], + }); + } +} diff --git a/library/sources/react-router/wrapRequestBodyParsing.ts b/library/sources/react-router/wrapRequestBodyParsing.ts new file mode 100644 index 000000000..4b81dfe13 --- /dev/null +++ b/library/sources/react-router/wrapRequestBodyParsing.ts @@ -0,0 +1,42 @@ +import { getContext, updateContext } from "../../agent/Context"; +import { formDataToPlainObject } from "../../helpers/formDataToPlainObject"; +import { createWrappedFunction, isWrapped } from "../../helpers/wrap"; + +/** + * Wrap the Request object's body parsing methods to update the context with the parsed body + * when any of the methods are called. + * This is needed because React Router uses the Fetch API Request object which has a stream + * body that can only be consumed once. We wrap the parsing methods to capture the result. + */ +export function wrapRequestBodyParsing(request: Request) { + request.formData = wrapBodyParsingFunction(request.formData); + request.json = wrapBodyParsingFunction(request.json); + request.text = wrapBodyParsingFunction(request.text); +} + +function wrapBodyParsingFunction(func: T) { + if (isWrapped(func)) { + return func; + } + + return createWrappedFunction(func, function parse(parser) { + return async function wrap() { + // @ts-expect-error No type for arguments + // eslint-disable-next-line prefer-rest-params + const returnValue = await parser.apply(this, arguments); + + if (returnValue) { + const context = getContext(); + if (context) { + if (returnValue instanceof FormData) { + updateContext(context, "body", formDataToPlainObject(returnValue)); + } else { + updateContext(context, "body", returnValue); + } + } + } + + return returnValue; + }; + }) as T; +}