Merge pull request #635 from AikidoSec/child-process-exec

Do not wrap exec as it internally uses execFile

Author: hansott

library/sinks/ChildProcess.monitor.test.ts

@@ -0,0 +1,62 @@
+import * as t from "tap";
+import { Context, runWithContext } from "../agent/Context";
+import { ChildProcess } from "./ChildProcess";
+import { execFile, execFileSync } from "child_process";
+import { createTestAgent } from "../helpers/createTestAgent";
+import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
+import * as FakeTimers from "@sinonjs/fake-timers";
+import { Token } from "../agent/api/Token";
+
+const unsafeContext: Context = {
+  remoteAddress: "::1",
+  method: "POST",
+  url: "http://localhost:4000",
+  query: {},
+  headers: {},
+  body: {
+    file: {
+      matches: "`echo .`",
+    },
+  },
+  cookies: {},
+  routeParams: {},
+  source: "express",
+  route: "/posts/:id",
+};
+
+t.test("it does not report the attack twice", async (t) => {
+  const clock = FakeTimers.install();
+
+  const api = new ReportingAPIForTesting();
+  const agent = createTestAgent({
+    block: false,
+    api: api,
+    token: new Token("test"),
+  });
+
+  agent.start([new ChildProcess()]);
+
+  const { exec } = require("child_process") as typeof import("child_process");
+
+  runWithContext(unsafeContext, () => {
+    exec("ls `echo .`", (err, stdout, stderr) => {}).unref();
+  });
+
+  clock.tick(60 * 1000);
+
+  const attacks = api.getEvents().filter((e) => e.type === "detected_attack");
+
+  t.match(attacks, [
+    {
+      type: "detected_attack",
+      attack: {
+        module: "child_process",
+        operation: "child_process.execFile",
+      },
+    },
+  ]);
+
+  t.same(attacks.length, 1);
+
+  clock.uninstall();
+});

library/sinks/ChildProcess.test.ts

@@ -95,7 +95,7 @@ t.test("it works", async (t) => {
   runWithContext(unsafeContext, () => {
     throws(
       () => exec("ls `echo .`", (err, stdout, stderr) => {}).unref(),
-      "Zen has blocked a shell injection: child_process.exec(...) originating from body.file.matches"
+      "Zen has blocked a shell injection: child_process.execFile(...) originating from body.file.matches"
     );
 
     throws(

library/sinks/ChildProcess.ts

@@ -19,12 +19,6 @@ const PATH_PREFIXES = [
 export class ChildProcess implements Wrapper {
   wrap(hooks: Hooks) {
     hooks.addBuiltinModule("child_process").onRequire((exports, pkgInfo) => {
-      wrapExport(exports, "exec", pkgInfo, {
-        kind: "exec_op",
-        inspectArgs: (args) => {
-          return this.inspectExec(args, "exec");
-        },
-      });
       wrapExport(exports, "execSync", pkgInfo, {
         kind: "exec_op",
         inspectArgs: (args) => {
@@ -44,6 +38,7 @@ export class ChildProcess implements Wrapper {
         },
       });
       wrapExport(exports, "execFile", pkgInfo, {
+        // Also protects exec, see https://github.com/nodejs/node/blob/main/lib/child_process.js
         kind: "exec_op",
         inspectArgs: (args) => {
           return this.inspectExecFile(args, "execFile");

library/sinks/Shelljs.test.ts

@@ -106,7 +106,7 @@ t.test("it detects async shell injections", async (t) => {
   if (error instanceof Error) {
     t.same(
       error.message,
-      "Zen has blocked a shell injection: child_process.exec(...) originating from body.myTitle"
+      "Zen has blocked a shell injection: child_process.execFile(...) originating from body.myTitle"
     );
   }
 
@@ -120,7 +120,7 @@ t.test("it detects async shell injections", async (t) => {
   if (error2 instanceof Error) {
     t.same(
       error2.message,
-      "Zen has blocked a shell injection: child_process.exec(...) originating from body.myTitle"
+      "Zen has blocked a shell injection: child_process.execFile(...) originating from body.myTitle"
     );
   }
 
@@ -134,7 +134,7 @@ t.test("it detects async shell injections", async (t) => {
   if (error3 instanceof Error) {
     t.same(
       error3.message,
-      "Zen has blocked a shell injection: child_process.exec(...) originating from body.myTitle"
+      "Zen has blocked a shell injection: child_process.execFile(...) originating from body.myTitle"
     );
   }
 });