Merge pull request #416 from AikidoSec/patch-fs-promises

Wrap require("fs").promises if "fs" is required

library/sinks/FileSystem.promises.test.ts

@@ -0,0 +1,46 @@
+import * as t from "tap";
+import { Context, runWithContext } from "../agent/Context";
+import { FileSystem } from "./FileSystem";
+import { createTestAgent } from "../helpers/createTestAgent";
+
+const unsafeContext: Context = {
+  remoteAddress: "::1",
+  method: "POST",
+  url: "http://localhost:4000",
+  query: {},
+  headers: {},
+  body: {
+    file: {
+      matches: "../test.txt",
+    },
+  },
+  cookies: {},
+  routeParams: {},
+  source: "express",
+  route: "/posts/:id",
+};
+
+t.test("when require('fs').promises is used", async (t) => {
+  const agent = createTestAgent();
+
+  agent.start([new FileSystem()]);
+
+  const { promises } = require("fs");
+
+  await runWithContext(unsafeContext, async () => {
+    const error = await t.rejects(() =>
+      promises.writeFile(
+        "../../test.txt",
+        "some other file content to test with",
+        { encoding: "utf-8" }
+      )
+    );
+    t.ok(error instanceof Error);
+    if (error instanceof Error) {
+      t.match(
+        error.message,
+        "Zen has blocked a path traversal attack: fs.writeFile(...) originating from body.file.matches"
+      );
+    }
+  });
+});

library/sinks/FileSystem.ts

@@ -2,6 +2,7 @@ import { getContext } from "../agent/Context";
 import { Hooks } from "../agent/hooks/Hooks";
 import { InterceptorResult } from "../agent/hooks/InterceptorResult";
 import { wrapExport } from "../agent/hooks/wrapExport";
+import { WrapPackageInfo } from "../agent/hooks/WrapPackageInfo";
 import { Wrapper } from "../agent/Wrapper";
 import { getSemverNodeVersion } from "../helpers/getNodeVersion";
 import { isVersionGreaterOrEqual } from "../helpers/isVersionGreaterOrEqual";
@@ -14,6 +15,8 @@ type FileSystemFunction = {
 };
 
 export class FileSystem implements Wrapper {
+  private patchedPromises = false;
+
   private inspectPath(
     args: unknown[],
     name: string,
@@ -91,12 +94,33 @@ export class FileSystem implements Wrapper {
     return functions;
   }
 
+  wrapPromises(exports: unknown, pkgInfo: WrapPackageInfo) {
+    if (this.patchedPromises) {
+      // `require("fs").promises.readFile` is the same as `require("fs/promises").readFile`
+      // We only need to wrap the promise version once
+      return;
+    }
+
+    const functions = this.getFunctions();
+    Object.keys(functions).forEach((name) => {
+      const { pathsArgs, promise } = functions[name];
+
+      if (promise) {
+        wrapExport(exports, name, pkgInfo, {
+          inspectArgs: (args) => this.inspectPath(args, name, pathsArgs),
+        });
+      }
+    });
+
+    this.patchedPromises = true;
+  }
+
   wrap(hooks: Hooks) {
     hooks.addBuiltinModule("fs").onRequire((exports, pkgInfo) => {
       const functions = this.getFunctions();
 
       Object.keys(functions).forEach((name) => {
-        const { pathsArgs, sync, promise } = functions[name];
+        const { pathsArgs, sync } = functions[name];
 
         wrapExport(exports, name, pkgInfo, {
           inspectArgs: (args) => {
@@ -119,24 +143,18 @@ export class FileSystem implements Wrapper {
           return this.inspectPath(args, "realpath.native", 1);
         },
       });
+
       wrapExport(exports.realpathSync, "native", pkgInfo, {
         inspectArgs: (args) => {
           return this.inspectPath(args, "realpathSync.native", 1);
         },
       });
-    });
-
-    hooks.addBuiltinModule("fs/promises").onRequire((exports, pkgInfo) => {
-      const functions = this.getFunctions();
-      Object.keys(functions).forEach((name) => {
-        const { pathsArgs, sync, promise } = functions[name];
 
-        if (promise) {
-          wrapExport(exports, name, pkgInfo, {
-            inspectArgs: (args) => this.inspectPath(args, name, pathsArgs),
-          });
-        }
-      });
+      this.wrapPromises(exports.promises, pkgInfo);
     });
+
+    hooks
+      .addBuiltinModule("fs/promises")
+      .onRequire((exports, pkgInfo) => this.wrapPromises(exports, pkgInfo));
   }
 }