diff --git a/library/helpers/isWindows.ts b/library/helpers/isWindows.ts new file mode 100644 index 000000000..3cc1885e2 --- /dev/null +++ b/library/helpers/isWindows.ts @@ -0,0 +1,3 @@ +export function isWindows() { + return process.platform === "win32"; +} diff --git a/library/sinks/Path.doubleWrapping.test.ts b/library/sinks/Path.doubleWrapping.test.ts new file mode 100644 index 000000000..ee9c22325 --- /dev/null +++ b/library/sinks/Path.doubleWrapping.test.ts @@ -0,0 +1,50 @@ +import { join, normalize, resolve } from "path/posix"; +import * as t from "tap"; +import { isWindows } from "../helpers/isWindows"; +import { isWrapped } from "../helpers/wrap"; +import { Path } from "./Path"; +import { createTestAgent } from "../helpers/createTestAgent"; + +t.test( + "it works", + { skip: isWindows() ? "path is not the same as path/posix" : false }, + async (t) => { + const agent = createTestAgent(); + + agent.start([new Path()]); + + const { join, resolve, normalize } = require("path/posix"); + + // Path required after path/posix + require("path"); + + const checkForDoubleWrapping = [join, resolve, normalize]; + for (const fn of checkForDoubleWrapping) { + if (isWrapped(fn) && isWrapped(fn.__original)) { + t.fail(`${fn.name} is double wrapped!`); + } + } + } +); + +t.test( + "it works", + { skip: !isWindows() ? "path is not the same as path/win32" : false }, + async (t) => { + const agent = createTestAgent(); + + agent.start([new Path()]); + + const { join, resolve, normalize } = require("path/win32"); + + // Path required after path/win32 + require("path"); + + const checkForDoubleWrapping = [join, resolve, normalize]; + for (const fn of checkForDoubleWrapping) { + if (isWrapped(fn) && isWrapped(fn.__original)) { + t.fail(`${fn.name} is double wrapped!`); + } + } + } +); diff --git a/library/sinks/Path.test.ts b/library/sinks/Path.test.ts index 52b646236..a0c891a4c 100644 --- a/library/sinks/Path.test.ts +++ b/library/sinks/Path.test.ts @@ -1,5 +1,6 @@ import * as t from "tap"; import { Context, runWithContext } from "../agent/Context"; +import { isWrapped } from "../helpers/wrap"; import { Path } from "./Path"; import { createTestAgent } from "../helpers/createTestAgent"; @@ -30,7 +31,8 @@ t.test("it works", async (t) => { agent.start([new Path()]); - const { join, resolve } = require("path"); + // Path required before path/posix and path/win32 + const { join, resolve, normalize } = require("path"); function safeCalls() { t.same(join("test.txt"), "test.txt"); @@ -147,4 +149,11 @@ t.test("it works", async (t) => { "Zen has blocked a path traversal attack: path.normalize(...) originating from body.file.matches" ); }); + + const checkForDoubleWrapping = [join, resolve, normalize]; + for (const fn of checkForDoubleWrapping) { + if (isWrapped(fn) && isWrapped(fn.__original)) { + t.fail(`${fn.name} is double wrapped!`); + } + } }); diff --git a/library/sinks/Path.ts b/library/sinks/Path.ts index 0b971781c..a1d19058b 100644 --- a/library/sinks/Path.ts +++ b/library/sinks/Path.ts @@ -3,9 +3,14 @@ import { Hooks } from "../agent/hooks/Hooks"; import { wrapExport } from "../agent/hooks/wrapExport"; import { WrapPackageInfo } from "../agent/hooks/WrapPackageInfo"; import { Wrapper } from "../agent/Wrapper"; +import { isWindows } from "../helpers/isWindows"; import { checkContextForPathTraversal } from "../vulnerabilities/path-traversal/checkContextForPathTraversal"; +import type * as path from "path"; export class Path implements Wrapper { + private patchedPosix = false; + private patchedWin32 = false; + private inspectPath(args: unknown[], operation: string) { const context = getContext(); @@ -34,19 +39,65 @@ export class Path implements Wrapper { return undefined; } - wrap(hooks: Hooks): void { + private wrapFunctions(exports: unknown, pkgInfo: WrapPackageInfo) { const functions = ["join", "resolve", "normalize"]; - const onRequire = (exports: any, pkgInfo: WrapPackageInfo) => { - for (const func of functions) { - wrapExport(exports, func, pkgInfo, { - inspectArgs: (args) => this.inspectPath(args, func), - }); - } - }; + for (const func of functions) { + wrapExport(exports, func, pkgInfo, { + inspectArgs: (args) => this.inspectPath(args, func), + }); + } + } + + private wrapMainModule(exports: typeof path, pkgInfo: WrapPackageInfo) { + // If `path/win32` or `path/posix` was not required before `path`, we should wrap the functions in `path` + if (!this.patchedWin32 && !this.patchedPosix) { + this.wrapFunctions(exports, pkgInfo); + } + + if (isWindows()) { + // `require("path").join` is the same as `require("path/win32").join` + this.patchedWin32 = true; + } else { + // `require("path").join` is the same as `require("path/posix").join` + this.patchedPosix = true; + } + + this.wrapPosix(exports.posix, pkgInfo); + this.wrapWin32(exports.win32, pkgInfo); + } + + private wrapPosix(exports: unknown, pkgInfo: WrapPackageInfo) { + if (this.patchedPosix) { + return; + } + + this.wrapFunctions(exports, pkgInfo); + + this.patchedPosix = true; + } + + private wrapWin32(exports: unknown, pkgInfo: WrapPackageInfo) { + if (this.patchedWin32) { + return; + } + + this.wrapFunctions(exports, pkgInfo); + + this.patchedWin32 = true; + } + + wrap(hooks: Hooks): void { + hooks + .addBuiltinModule("path") + .onRequire((exports, pkgInfo) => this.wrapMainModule(exports, pkgInfo)); + + hooks + .addBuiltinModule("path/posix") + .onRequire((exports, pkgInfo) => this.wrapPosix(exports, pkgInfo)); - hooks.addBuiltinModule("path").onRequire(onRequire); - hooks.addBuiltinModule("path/posix").onRequire(onRequire); - hooks.addBuiltinModule("path/win32").onRequire(onRequire); + hooks + .addBuiltinModule("path/win32") + .onRequire((exports, pkgInfo) => this.wrapWin32(exports, pkgInfo)); } }