From 101157fc5ffa51c7108f909eaaae66f6b1ee9cf8 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 27 May 2025 22:48:02 +0100 Subject: [PATCH 01/26] feat(probes): add isUnsafeSpawn Signed-off-by: Tony Gorez --- docs/unsafe-spawn.md | 1 + src/ProbeRunner.js | 4 +- src/probes/isUnsafeSpawn.js | 69 +++++++++++++++++++++++++++++++ src/warnings.js | 5 +++ test/probes/isUnsafeSpawn.spec.js | 52 +++++++++++++++++++++++ types/warnings.d.ts | 3 +- 6 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 docs/unsafe-spawn.md create mode 100644 src/probes/isUnsafeSpawn.js create mode 100644 test/probes/isUnsafeSpawn.spec.js diff --git a/docs/unsafe-spawn.md b/docs/unsafe-spawn.md new file mode 100644 index 0000000..0db07ff --- /dev/null +++ b/docs/unsafe-spawn.md @@ -0,0 +1 @@ +# Unsafe spawn diff --git a/src/ProbeRunner.js b/src/ProbeRunner.js index fcf01c1..ca8b32d 100644 --- a/src/ProbeRunner.js +++ b/src/ProbeRunner.js @@ -13,6 +13,7 @@ import isBinaryExpression from "./probes/isBinaryExpression.js"; import isArrayExpression from "./probes/isArrayExpression.js"; import isESMExport from "./probes/isESMExport.js"; import isFetch from "./probes/isFetch.js"; +import isUnsafeSpawn from "./probes/isUnsafeSpawn.js" // Import Internal Dependencies import { SourceFile } from "./SourceFile.js"; @@ -50,7 +51,8 @@ export class ProbeRunner { isImportDeclaration, isWeakCrypto, isBinaryExpression, - isArrayExpression + isArrayExpression, + isUnsafeSpawn ]; /** diff --git a/src/probes/isUnsafeSpawn.js b/src/probes/isUnsafeSpawn.js new file mode 100644 index 0000000..b9aca00 --- /dev/null +++ b/src/probes/isUnsafeSpawn.js @@ -0,0 +1,69 @@ +// Import Internal Dependencies +import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils"; +import { ProbeSignals } from "../ProbeRunner.js"; + +/** + * @description Detect spawn commands containing csrutil + * @example + * child_process.spawn("csrutil", ["status"]); + * require("child_process").spawn("csrutil", ["disable"]); + * const { spawn } = require("child_process"); + * spawn("csrutil", ["status"]); + */ +function validateNode(node, { tracer }) { + if (node.type !== "CallExpression" || node.arguments.length === 0) { + return [false]; + } + + // Direct: child_process.spawn(...) or require("child_process").spawn(...) + if ( + node.callee.type === "MemberExpression" && + node.callee.property.type === "Identifier" && + node.callee.property.name === "spawn" + ) { + // child_process.spawn(...) + if ( + node.callee.object.type === "Identifier" && + node.callee.object.name === "child_process" + ) { + return [true]; + } + // require("child_process").spawn(...) + if ( + node.callee.object.type === "CallExpression" && + node.callee.object.callee.type === "Identifier" && + node.callee.object.callee.name === "require" && + node.callee.object.arguments.length === 1 && + node.callee.object.arguments[0].type === "Literal" && + node.callee.object.arguments[0].value === "child_process" + ) { + return [true]; + } + } + + return [false]; +} + +function main(node, options) { + const { sourceFile } = options; + + // Get the first argument of spawn which should be the command + const commandArg = node.arguments[0]; + if (!commandArg || commandArg.type !== "Literal") { + return null; + } + + const command = commandArg.value; + if (typeof command === "string" && command.includes("csrutil")) { + sourceFile.addWarning("unsafe-spawn", command, node.loc); + return ProbeSignals.Skip; + } + + return null; +} + +export default { + name: "isUnsafeSpwan", + validateNode, + main +}; diff --git a/src/warnings.js b/src/warnings.js index e8deebe..57e54b2 100644 --- a/src/warnings.js +++ b/src/warnings.js @@ -50,6 +50,11 @@ export const warnings = Object.freeze({ i18n: "sast_warnings.shady_link", severity: "Warning", experimental: false + }, + "unsafe-spawn": { + i18n: "sast_warnings.unsafe-spawn", + severity: "Warning", + experimental: true } }); diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js new file mode 100644 index 0000000..409e15f --- /dev/null +++ b/test/probes/isUnsafeSpawn.spec.js @@ -0,0 +1,52 @@ +// Import Node.js dependencies +import { test } from "node:test"; +import assert from "node:assert"; + +// Import Internal Dependencies +import { parseScript, getSastAnalysis } from "../utils/index.js"; +import isUnsafeSpawn from "../../src/probes/isUnsafeSpawn.js"; + +// CONSTANTS +const kWarningUnsafeSpawn = "unsafe-spawn"; + +// test("should detect csrutil spawn command", () => { +// const str = ` +// const { spawn } = require("child_process"); +// spawn("csrutil", ["status"]); +// `; +// +// const ast = parseScript(str); +// const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) +// .execute(ast.body); +// +// const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); +// assert.equal(result.kind, kWarningUnsafeSpawn); +// assert.equal(result.value, "csrutil"); +// }); + +test("should detect csrutil spawn command with require", () => { + const str = ` + require("child_process").spawn("csrutil", ["disable"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); + assert.equal(result.kind, kWarningUnsafeSpawn); + assert.equal(result.value, "csrutil"); +}); + +test("should not detect non-csrutil spawn command", () => { + const str = ` + const { spawn } = require("child_process"); + spawn("ls", ["-la"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + .execute(ast.body); + + assert.equal(sastAnalysis.warnings().length, 0); +}); diff --git a/types/warnings.d.ts b/types/warnings.d.ts index 32c092c..4f0e610 100644 --- a/types/warnings.d.ts +++ b/types/warnings.d.ts @@ -16,7 +16,8 @@ type WarningNameWithValue = "parsing-error" | "suspicious-file" | "obfuscated-code" | "weak-crypto" -| "shady-link"; +| "shady-link" +| "unsafe-spwan"; type WarningName = WarningNameWithValue | "unsafe-import"; type WarningLocation = [[number, number], [number, number]]; From 0f8a00bf0cc22dbd0b51416530ec22320fe92edf Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 10 Jun 2025 08:30:10 +0200 Subject: [PATCH 02/26] refactor(probes): add isUnsafeCommand helper Signed-off-by: Tony Gorez --- src/probes/isUnsafeSpawn.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/probes/isUnsafeSpawn.js b/src/probes/isUnsafeSpawn.js index b9aca00..757bec6 100644 --- a/src/probes/isUnsafeSpawn.js +++ b/src/probes/isUnsafeSpawn.js @@ -2,6 +2,12 @@ import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils"; import { ProbeSignals } from "../ProbeRunner.js"; +const kUnsafeCommands = ["csrutil"] + +function isUnsafeCommand(command) { + return kUnsafeCommands.filter(unsafeCommand => command.includes(unsafeCommand)); +} + /** * @description Detect spawn commands containing csrutil * @example @@ -47,14 +53,13 @@ function validateNode(node, { tracer }) { function main(node, options) { const { sourceFile } = options; - // Get the first argument of spawn which should be the command const commandArg = node.arguments[0]; if (!commandArg || commandArg.type !== "Literal") { return null; } const command = commandArg.value; - if (typeof command === "string" && command.includes("csrutil")) { + if (typeof command === "string" && isUnsafeCommand(command)) { sourceFile.addWarning("unsafe-spawn", command, node.loc); return ProbeSignals.Skip; } From a9e7c1ef954c970bf44dcb38ebc122cefbe73e5b Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 10 Jun 2025 08:49:26 +0200 Subject: [PATCH 03/26] feat(probes): handle standalone spawn func Signed-off-by: Tony Gorez --- src/probes/isUnsafeSpawn.js | 12 ++++++++++-- test/probes/isUnsafeSpawn.spec.js | 28 ++++++++++++++-------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/probes/isUnsafeSpawn.js b/src/probes/isUnsafeSpawn.js index 757bec6..6416212 100644 --- a/src/probes/isUnsafeSpawn.js +++ b/src/probes/isUnsafeSpawn.js @@ -5,7 +5,7 @@ import { ProbeSignals } from "../ProbeRunner.js"; const kUnsafeCommands = ["csrutil"] function isUnsafeCommand(command) { - return kUnsafeCommands.filter(unsafeCommand => command.includes(unsafeCommand)); + return !!kUnsafeCommands.find(unsafeCommand => command.includes(unsafeCommand)); } /** @@ -21,7 +21,15 @@ function validateNode(node, { tracer }) { return [false]; } - // Direct: child_process.spawn(...) or require("child_process").spawn(...) + // const { spawn } = require("child_process"); + // spawn("csrutil", ["status"]); + if (node.type === "CallExpression" && + node.callee.type === "Identifier" && + node.callee.name === "spawn") { + return [true] + } + + // child_process.spawn(...) or require("child_process").spawn(...) if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js index 409e15f..0bfa654 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeSpawn.spec.js @@ -9,20 +9,20 @@ import isUnsafeSpawn from "../../src/probes/isUnsafeSpawn.js"; // CONSTANTS const kWarningUnsafeSpawn = "unsafe-spawn"; -// test("should detect csrutil spawn command", () => { -// const str = ` -// const { spawn } = require("child_process"); -// spawn("csrutil", ["status"]); -// `; -// -// const ast = parseScript(str); -// const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) -// .execute(ast.body); -// -// const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); -// assert.equal(result.kind, kWarningUnsafeSpawn); -// assert.equal(result.value, "csrutil"); -// }); +test("should detect csrutil spawn command", () => { + const str = ` + const { spawn } = require("child_process"); + spawn("csrutil", ["status"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); + assert.equal(result.kind, kWarningUnsafeSpawn); + assert.equal(result.value, "csrutil"); +}); test("should detect csrutil spawn command with require", () => { const str = ` From 27a5848eab3caa2fde54b686ba776f576b3969ca Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 10 Jun 2025 08:52:26 +0200 Subject: [PATCH 04/26] test: add member Signed-off-by: Tony Gorez --- test/probes/isUnsafeSpawn.spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js index 0bfa654..271c031 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeSpawn.spec.js @@ -38,6 +38,20 @@ test("should detect csrutil spawn command with require", () => { assert.equal(result.value, "csrutil"); }); +test("should detect csrutil spawn command as child_process member", () => { + const str = ` + child_process.spawn("csrutil", ["disable"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); + assert.equal(result.kind, kWarningUnsafeSpawn); + assert.equal(result.value, "csrutil"); +}); + test("should not detect non-csrutil spawn command", () => { const str = ` const { spawn } = require("child_process"); From 886827a973f4ef6dd132d1c7c36b69d6202ded49 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 10 Jun 2025 09:02:20 +0200 Subject: [PATCH 05/26] test: add last (missing) case Signed-off-by: Tony Gorez --- test/probes/isUnsafeSpawn.spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js index 271c031..eb45177 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeSpawn.spec.js @@ -24,6 +24,21 @@ test("should detect csrutil spawn command", () => { assert.equal(result.value, "csrutil"); }); +test("should detect hidden csrutil spawn command", () => { + const str = ` + const { spawn: hide } = require("child_process"); + hide("csrutil", ["status"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); + assert.equal(result.kind, kWarningUnsafeSpawn); + assert.equal(result.value, "csrutil"); +}); + test("should detect csrutil spawn command with require", () => { const str = ` require("child_process").spawn("csrutil", ["disable"]); From 3660cce0b108822c805594844a8c1539d18427a3 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Thu, 12 Jun 2025 21:49:12 +0200 Subject: [PATCH 06/26] test: remove child_process member test and skip hide one Signed-off-by: Tony Gorez --- test/probes/isUnsafeSpawn.spec.js | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js index eb45177..93855a8 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeSpawn.spec.js @@ -24,7 +24,8 @@ test("should detect csrutil spawn command", () => { assert.equal(result.value, "csrutil"); }); -test("should detect hidden csrutil spawn command", () => { +// TODO: de-skip when the tracer would be ready +test.skip("should detect hidden csrutil spawn command", () => { const str = ` const { spawn: hide } = require("child_process"); hide("csrutil", ["status"]); @@ -53,20 +54,6 @@ test("should detect csrutil spawn command with require", () => { assert.equal(result.value, "csrutil"); }); -test("should detect csrutil spawn command as child_process member", () => { - const str = ` - child_process.spawn("csrutil", ["disable"]); - `; - - const ast = parseScript(str); - const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) - .execute(ast.body); - - const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); - assert.equal(result.kind, kWarningUnsafeSpawn); - assert.equal(result.value, "csrutil"); -}); - test("should not detect non-csrutil spawn command", () => { const str = ` const { spawn } = require("child_process"); From 9db1db33206a1ae86636bc89adea93312c6eff08 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Fri, 13 Jun 2025 09:16:34 +0200 Subject: [PATCH 07/26] fix: lint Signed-off-by: Tony Gorez --- src/ProbeRunner.js | 2 +- src/probes/isUnsafeSpawn.js | 14 +++++++------- test/probes/isUnsafeSpawn.spec.js | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ProbeRunner.js b/src/ProbeRunner.js index bad77d6..9b4c21e 100644 --- a/src/ProbeRunner.js +++ b/src/ProbeRunner.js @@ -13,7 +13,7 @@ import isBinaryExpression from "./probes/isBinaryExpression.js"; import isArrayExpression from "./probes/isArrayExpression.js"; import isESMExport from "./probes/isESMExport.js"; import isFetch from "./probes/isFetch.js"; -import isUnsafeSpawn from "./probes/isUnsafeSpawn.js" +import isUnsafeSpawn from "./probes/isUnsafeSpawn.js"; /** * @typedef {import('./SourceFile.js').SourceFile} SourceFile diff --git a/src/probes/isUnsafeSpawn.js b/src/probes/isUnsafeSpawn.js index 6416212..583ef69 100644 --- a/src/probes/isUnsafeSpawn.js +++ b/src/probes/isUnsafeSpawn.js @@ -1,11 +1,10 @@ // Import Internal Dependencies -import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils"; import { ProbeSignals } from "../ProbeRunner.js"; -const kUnsafeCommands = ["csrutil"] +const kUnsafeCommands = ["csrutil"]; function isUnsafeCommand(command) { - return !!kUnsafeCommands.find(unsafeCommand => command.includes(unsafeCommand)); + return Boolean(kUnsafeCommands.find((unsafeCommand) => command.includes(unsafeCommand))); } /** @@ -16,7 +15,7 @@ function isUnsafeCommand(command) { * const { spawn } = require("child_process"); * spawn("csrutil", ["status"]); */ -function validateNode(node, { tracer }) { +function validateNode(node) { if (node.type !== "CallExpression" || node.arguments.length === 0) { return [false]; } @@ -24,9 +23,9 @@ function validateNode(node, { tracer }) { // const { spawn } = require("child_process"); // spawn("csrutil", ["status"]); if (node.type === "CallExpression" && - node.callee.type === "Identifier" && - node.callee.name === "spawn") { - return [true] + node.callee.type === "Identifier" && + node.callee.name === "spawn") { + return [true]; } // child_process.spawn(...) or require("child_process").spawn(...) @@ -69,6 +68,7 @@ function main(node, options) { const command = commandArg.value; if (typeof command === "string" && isUnsafeCommand(command)) { sourceFile.addWarning("unsafe-spawn", command, node.loc); + return ProbeSignals.Skip; } diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeSpawn.spec.js index 93855a8..cae0129 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeSpawn.spec.js @@ -1,4 +1,4 @@ -// Import Node.js dependencies +// Import Node.js Dependencies import { test } from "node:test"; import assert from "node:assert"; From 2ede3c14bdb4e5ae8f7d73e6d9bb5be7de95aba0 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Fri, 13 Jun 2025 09:23:58 +0200 Subject: [PATCH 08/26] doc: add unsafe-spawn Signed-off-by: Tony Gorez --- docs/unsafe-spawn.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/unsafe-spawn.md b/docs/unsafe-spawn.md index 0db07ff..94ca678 100644 --- a/docs/unsafe-spawn.md +++ b/docs/unsafe-spawn.md @@ -1 +1,21 @@ # Unsafe spawn + +| Code | Severity | i18n | Experimental | +| --- | --- | --- | :-: | +| unsafe-spwan | `Warning` | `sast_warnings.unsafe_spawn` | ✅ | + +## Introduction + +This warning identifies potentially dangerous use of the `spawn()` function from the `child_process` module. +Spawning system-level commands can introduce security risks, especially if user-controlled input is involved or if the +command itself is sensitive (e.g., tools that query or change system configurations). + +> [!NOTE] +> This rule is experimental. The list of suspicious commands is not exhaustive and will evolve over time. + +## Example + +```js +const { spawn } = require("child_process"); +spawn("csrutil", ["status"]); +``` From 0a1502a90ae7237f161165712b814b3bf33e9dbb Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Fri, 13 Jun 2025 09:26:56 +0200 Subject: [PATCH 09/26] doc: update readme Signed-off-by: Tony Gorez --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index ed6c018..7bd7409 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ type WarningName = "parsing-error" | "obfuscated-code" | "weak-crypto" | "unsafe-import" +| "unsafe-spawn" | "shady-link"; declare const warnings: Record Date: Mon, 16 Jun 2025 22:37:19 +0200 Subject: [PATCH 10/26] refactor(probes): switch to unsafe-command Signed-off-by: Tony Gorez --- README.md | 4 +-- docs/{unsafe-spawn.md => unsafe-command.md} | 8 +++--- src/ProbeRunner.js | 4 +-- .../{isUnsafeSpawn.js => isUnsafeCommand.js} | 27 ++++++++++++------- src/warnings.js | 4 +-- ...eSpawn.spec.js => isUnsafeCommand.spec.js} | 24 ++++++++--------- types/warnings.d.ts | 2 +- workspaces/sec-literal/package.json | 1 + 8 files changed, 42 insertions(+), 32 deletions(-) rename docs/{unsafe-spawn.md => unsafe-command.md} (72%) rename src/probes/{isUnsafeSpawn.js => isUnsafeCommand.js} (72%) rename test/probes/{isUnsafeSpawn.spec.js => isUnsafeCommand.spec.js} (64%) diff --git a/README.md b/README.md index 7bd7409..7791ab1 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ type WarningName = "parsing-error" | "obfuscated-code" | "weak-crypto" | "unsafe-import" -| "unsafe-spawn" +| "unsafe-command" | "shady-link"; declare const warnings: Record [!NOTE] diff --git a/src/ProbeRunner.js b/src/ProbeRunner.js index 9b4c21e..cbf5267 100644 --- a/src/ProbeRunner.js +++ b/src/ProbeRunner.js @@ -13,7 +13,7 @@ import isBinaryExpression from "./probes/isBinaryExpression.js"; import isArrayExpression from "./probes/isArrayExpression.js"; import isESMExport from "./probes/isESMExport.js"; import isFetch from "./probes/isFetch.js"; -import isUnsafeSpawn from "./probes/isUnsafeSpawn.js"; +import isUnsafeCommand from "./probes/isUnsafeCommand.js"; /** * @typedef {import('./SourceFile.js').SourceFile} SourceFile @@ -53,7 +53,7 @@ export class ProbeRunner { isWeakCrypto, isBinaryExpression, isArrayExpression, - isUnsafeSpawn + isUnsafeCommand ]; /** diff --git a/src/probes/isUnsafeSpawn.js b/src/probes/isUnsafeCommand.js similarity index 72% rename from src/probes/isUnsafeSpawn.js rename to src/probes/isUnsafeCommand.js index 583ef69..b64b87e 100644 --- a/src/probes/isUnsafeSpawn.js +++ b/src/probes/isUnsafeCommand.js @@ -8,33 +8,41 @@ function isUnsafeCommand(command) { } /** - * @description Detect spawn commands containing csrutil + * @description Detect spawn or exec unsafe commands * @example * child_process.spawn("csrutil", ["status"]); + * * require("child_process").spawn("csrutil", ["disable"]); - * const { spawn } = require("child_process"); - * spawn("csrutil", ["status"]); + * + * const { exec } = require("child_process"); + * exec("csrutil status"); */ function validateNode(node) { if (node.type !== "CallExpression" || node.arguments.length === 0) { return [false]; } - // const { spawn } = require("child_process"); - // spawn("csrutil", ["status"]); + // const { spawn } = require("child_process"); + // spawn("...", ["..."]); + // or + // const { exec } = require("child_process"); + // exec(...); if (node.type === "CallExpression" && node.callee.type === "Identifier" && - node.callee.name === "spawn") { + (node.callee.name === "spawn" || node.callee.name === "exec") + ) { return [true]; } // child_process.spawn(...) or require("child_process").spawn(...) + // child_process.exec(...) or require("child_process").exec(...) if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && - node.callee.property.name === "spawn" + (node.callee.property.name === "spawn" || node.callee.property.name == "exec") ) { // child_process.spawn(...) + // child_process.exec(...) if ( node.callee.object.type === "Identifier" && node.callee.object.name === "child_process" @@ -42,6 +50,7 @@ function validateNode(node) { return [true]; } // require("child_process").spawn(...) + // require("child_process").exec(...) if ( node.callee.object.type === "CallExpression" && node.callee.object.callee.type === "Identifier" && @@ -67,7 +76,7 @@ function main(node, options) { const command = commandArg.value; if (typeof command === "string" && isUnsafeCommand(command)) { - sourceFile.addWarning("unsafe-spawn", command, node.loc); + sourceFile.addWarning("unsafe-command", command, node.loc); return ProbeSignals.Skip; } @@ -76,7 +85,7 @@ function main(node, options) { } export default { - name: "isUnsafeSpwan", + name: "isUnsafeCommand", validateNode, main }; diff --git a/src/warnings.js b/src/warnings.js index f5dbf26..7af9ab0 100644 --- a/src/warnings.js +++ b/src/warnings.js @@ -52,8 +52,8 @@ export const warnings = Object.freeze({ severity: "Warning", experimental: false }, - "unsafe-spawn": { - i18n: "sast_warnings.unsafe-spawn", + "unsafe-command": { + i18n: "sast_warnings.unsafe-command", severity: "Warning", experimental: true } diff --git a/test/probes/isUnsafeSpawn.spec.js b/test/probes/isUnsafeCommand.spec.js similarity index 64% rename from test/probes/isUnsafeSpawn.spec.js rename to test/probes/isUnsafeCommand.spec.js index cae0129..307ea1d 100644 --- a/test/probes/isUnsafeSpawn.spec.js +++ b/test/probes/isUnsafeCommand.spec.js @@ -4,10 +4,10 @@ import assert from "node:assert"; // Import Internal Dependencies import { parseScript, getSastAnalysis } from "../utils/index.js"; -import isUnsafeSpawn from "../../src/probes/isUnsafeSpawn.js"; +import isUnsafeCommand from "../../src/probes/isUnsafeCommand.js"; // CONSTANTS -const kWarningUnsafeSpawn = "unsafe-spawn"; +const kWarningUnsafeCommand = "unsafe-command"; test("should detect csrutil spawn command", () => { const str = ` @@ -16,11 +16,11 @@ test("should detect csrutil spawn command", () => { `; const ast = parseScript(str); - const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) .execute(ast.body); - const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); - assert.equal(result.kind, kWarningUnsafeSpawn); + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); assert.equal(result.value, "csrutil"); }); @@ -32,11 +32,11 @@ test.skip("should detect hidden csrutil spawn command", () => { `; const ast = parseScript(str); - const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) .execute(ast.body); - const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); - assert.equal(result.kind, kWarningUnsafeSpawn); + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); assert.equal(result.value, "csrutil"); }); @@ -46,11 +46,11 @@ test("should detect csrutil spawn command with require", () => { `; const ast = parseScript(str); - const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) .execute(ast.body); - const result = sastAnalysis.getWarning(kWarningUnsafeSpawn); - assert.equal(result.kind, kWarningUnsafeSpawn); + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); assert.equal(result.value, "csrutil"); }); @@ -61,7 +61,7 @@ test("should not detect non-csrutil spawn command", () => { `; const ast = parseScript(str); - const sastAnalysis = getSastAnalysis(str, isUnsafeSpawn) + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) .execute(ast.body); assert.equal(sastAnalysis.warnings().length, 0); diff --git a/types/warnings.d.ts b/types/warnings.d.ts index 4f0e610..aa471ca 100644 --- a/types/warnings.d.ts +++ b/types/warnings.d.ts @@ -17,7 +17,7 @@ type WarningNameWithValue = "parsing-error" | "obfuscated-code" | "weak-crypto" | "shady-link" -| "unsafe-spwan"; +| "unsafe-command"; type WarningName = WarningNameWithValue | "unsafe-import"; type WarningLocation = [[number, number], [number, number]]; diff --git a/workspaces/sec-literal/package.json b/workspaces/sec-literal/package.json index 75915d3..6400aae 100644 --- a/workspaces/sec-literal/package.json +++ b/workspaces/sec-literal/package.json @@ -32,6 +32,7 @@ "homepage": "https://github.com/NodeSecure/js-x-ray/tree/master/workspaces/sec-literal#readme", "dependencies": { "frequency-set": "^1.0.2", + "get-east-asian-width": "^1.3.0", "is-base64": "^1.1.0", "is-svg": "^6.0.0", "string-width": "^7.0.0" From 7d9f5978c2d6c437b21acb0d0cfd401bfd190fa8 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:39:54 +0200 Subject: [PATCH 11/26] fix: lint Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index b64b87e..59a5a84 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -39,7 +39,7 @@ function validateNode(node) { if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && - (node.callee.property.name === "spawn" || node.callee.property.name == "exec") + (node.callee.property.name === "spawn" || node.callee.property.name === "exec") ) { // child_process.spawn(...) // child_process.exec(...) From 656e4f78bd132a69acdf8153b067e5f9491b25e6 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:40:47 +0200 Subject: [PATCH 12/26] test(probes): switch assert to strict Signed-off-by: Tony Gorez --- test/probes/isWeakCrypto.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/probes/isWeakCrypto.spec.js b/test/probes/isWeakCrypto.spec.js index 4343fc4..5727758 100644 --- a/test/probes/isWeakCrypto.spec.js +++ b/test/probes/isWeakCrypto.spec.js @@ -1,7 +1,7 @@ // Import Node.js Dependencies import { readFileSync, promises as fs } from "node:fs"; import { test } from "node:test"; -import assert from "node:assert"; +import assert from "node:assert/strict"; // Import Internal Dependencies import { AstAnalyser } from "../../index.js"; From 2d78a19a57927966d21c4f15a011bfc6f8d3705e Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:42:11 +0200 Subject: [PATCH 13/26] fix: add missing comment Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 59a5a84..226b31c 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -1,6 +1,7 @@ // Import Internal Dependencies import { ProbeSignals } from "../ProbeRunner.js"; +// CONSTANTS const kUnsafeCommands = ["csrutil"]; function isUnsafeCommand(command) { From 58e1e9a4abe2d19fd09ce49c1f5dd12a6f13b989 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:42:58 +0200 Subject: [PATCH 14/26] refactor(probes): simplify isUnsafeCommand helper Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 226b31c..174f5d1 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -5,7 +5,7 @@ import { ProbeSignals } from "../ProbeRunner.js"; const kUnsafeCommands = ["csrutil"]; function isUnsafeCommand(command) { - return Boolean(kUnsafeCommands.find((unsafeCommand) => command.includes(unsafeCommand))); + return kUnsafeCommands.some((unsafeCommand) => command.includes(unsafeCommand)); } /** From d677a790de536f035f790660be742b7715f96494 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:44:21 +0200 Subject: [PATCH 15/26] doc: fix path in readme Signed-off-by: Tony Gorez --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7791ab1..7ddc490 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ This section describe all the possible warnings returned by JSXRay. Click on the | [unsafe-import](./docs/unsafe-import.md) | ❌ | Unable to follow an import (require, require.resolve) statement/expr. | | [unsafe-regex](./docs/unsafe-regex.md) | ❌ | A RegEx as been detected as unsafe and may be used for a ReDoS Attack. | | [unsafe-stmt](./docs//unsafe-stmt.md) | ❌ | Usage of dangerous statement like `eval()` or `Function("")`. | -| [unsafe-command](./docs//unsafe-command.md) | ❌ | Usage of suspicious commands in `spawn()` or `exec()`.| +| [unsafe-command](./docs/unsafe-command.md) | ❌ | Usage of suspicious commands in `spawn()` or `exec()`.| | [encoded-literal](./docs/encoded-literal.md) | ❌ | An encoded literal has been detected (it can be an hexa value, unicode sequence or a base64 string) | | [short-identifiers](./docs/short-identifiers.md) | ❌ | This mean that all identifiers has an average length below 1.5. | | [suspicious-literal](./docs/suspicious-literal.md) | ❌ | A suspicious literal has been found in the source code. | From 0265b77d00372ed988f4ae384470927d10f20f25 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:49:51 +0200 Subject: [PATCH 16/26] test(probes): add exec cases Signed-off-by: Tony Gorez --- test/probes/isUnsafeCommand.spec.js | 64 ++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/test/probes/isUnsafeCommand.spec.js b/test/probes/isUnsafeCommand.spec.js index 307ea1d..20ef885 100644 --- a/test/probes/isUnsafeCommand.spec.js +++ b/test/probes/isUnsafeCommand.spec.js @@ -9,6 +9,8 @@ import isUnsafeCommand from "../../src/probes/isUnsafeCommand.js"; // CONSTANTS const kWarningUnsafeCommand = "unsafe-command"; +// Spawn + test("should detect csrutil spawn command", () => { const str = ` const { spawn } = require("child_process"); @@ -54,7 +56,7 @@ test("should detect csrutil spawn command with require", () => { assert.equal(result.value, "csrutil"); }); -test("should not detect non-csrutil spawn command", () => { +test("should not detect non suspicious spawned command", () => { const str = ` const { spawn } = require("child_process"); spawn("ls", ["-la"]); @@ -66,3 +68,63 @@ test("should not detect non-csrutil spawn command", () => { assert.equal(sastAnalysis.warnings().length, 0); }); + +// Exec + +test("should detect csrutil exec command", () => { + const str = ` + const { exec } = require("child_process"); + exec("csrutil status"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +// TODO: de-skip when the tracer would be ready +test.skip("should detect hidden csrutil spawn command", () => { + const str = ` + const { exec: hide } = require("child_process"); + exec("csrutil status"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +test("should detect csrutil spawn command with require", () => { + const str = ` + require("child_process").exec("csrutil disable"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil disable"); +}); + +test("should not detect non suspicious exec-ed command", () => { + const str = ` + const { exec } = require("child_process"); + exec("ls -la"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + assert.equal(sastAnalysis.warnings().length, 0); +}); From b0975f33a7dfdb61edf13acc2c4bdfbefded5687 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Mon, 16 Jun 2025 22:54:25 +0200 Subject: [PATCH 17/26] fix: remove deps Signed-off-by: Tony Gorez --- workspaces/sec-literal/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/sec-literal/package.json b/workspaces/sec-literal/package.json index 6400aae..75915d3 100644 --- a/workspaces/sec-literal/package.json +++ b/workspaces/sec-literal/package.json @@ -32,7 +32,6 @@ "homepage": "https://github.com/NodeSecure/js-x-ray/tree/master/workspaces/sec-literal#readme", "dependencies": { "frequency-set": "^1.0.2", - "get-east-asian-width": "^1.3.0", "is-base64": "^1.1.0", "is-svg": "^6.0.0", "string-width": "^7.0.0" From 08d05fbfb703ca9d3416641098473e71b49c3169 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 17 Jun 2025 09:16:11 +0200 Subject: [PATCH 18/26] fix(probes): rebuild full command for spawned case Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 20 ++++++++++++++++---- test/probes/isUnsafeCommand.spec.js | 6 +++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 174f5d1..3eb927f 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -32,7 +32,7 @@ function validateNode(node) { node.callee.type === "Identifier" && (node.callee.name === "spawn" || node.callee.name === "exec") ) { - return [true]; + return [true, node.callee.name]; } // child_process.spawn(...) or require("child_process").spawn(...) @@ -48,7 +48,7 @@ function validateNode(node) { node.callee.object.type === "Identifier" && node.callee.object.name === "child_process" ) { - return [true]; + return [true, node.callee.property.name]; } // require("child_process").spawn(...) // require("child_process").exec(...) @@ -60,7 +60,7 @@ function validateNode(node) { node.callee.object.arguments[0].type === "Literal" && node.callee.object.arguments[0].value === "child_process" ) { - return [true]; + return [true, node.callee.property.name]; } } @@ -75,8 +75,20 @@ function main(node, options) { return null; } - const command = commandArg.value; + let command = commandArg.value; if (typeof command === "string" && isUnsafeCommand(command)) { + // Spwaned command arguments are filled into an Array + // as second arguments. This is why we should add them + // manually to the command string. + if (options.data === "spawn") { + const args = node.arguments.at(1); + if (args && Array.isArray(args.elements)) { + args.elements.forEach((element) => { + command += ` ${element.value}` + }) + } + } + sourceFile.addWarning("unsafe-command", command, node.loc); return ProbeSignals.Skip; diff --git a/test/probes/isUnsafeCommand.spec.js b/test/probes/isUnsafeCommand.spec.js index 20ef885..f36b45c 100644 --- a/test/probes/isUnsafeCommand.spec.js +++ b/test/probes/isUnsafeCommand.spec.js @@ -23,7 +23,7 @@ test("should detect csrutil spawn command", () => { const result = sastAnalysis.getWarning(kWarningUnsafeCommand); assert.equal(result.kind, kWarningUnsafeCommand); - assert.equal(result.value, "csrutil"); + assert.equal(result.value, "csrutil status"); }); // TODO: de-skip when the tracer would be ready @@ -39,7 +39,7 @@ test.skip("should detect hidden csrutil spawn command", () => { const result = sastAnalysis.getWarning(kWarningUnsafeCommand); assert.equal(result.kind, kWarningUnsafeCommand); - assert.equal(result.value, "csrutil"); + assert.equal(result.value, "csrutil status"); }); test("should detect csrutil spawn command with require", () => { @@ -53,7 +53,7 @@ test("should detect csrutil spawn command with require", () => { const result = sastAnalysis.getWarning(kWarningUnsafeCommand); assert.equal(result.kind, kWarningUnsafeCommand); - assert.equal(result.value, "csrutil"); + assert.equal(result.value, "csrutil disable"); }); test("should not detect non suspicious spawned command", () => { From aa08db9b23cb2a60987136598c26595c80899dbc Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Tue, 17 Jun 2025 09:41:28 +0200 Subject: [PATCH 19/26] test: use strict assert and revert unrelated one Signed-off-by: Tony Gorez --- test/probes/isUnsafeCommand.spec.js | 2 +- test/probes/isWeakCrypto.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/probes/isUnsafeCommand.spec.js b/test/probes/isUnsafeCommand.spec.js index f36b45c..fca906c 100644 --- a/test/probes/isUnsafeCommand.spec.js +++ b/test/probes/isUnsafeCommand.spec.js @@ -1,6 +1,6 @@ // Import Node.js Dependencies import { test } from "node:test"; -import assert from "node:assert"; +import assert from "node:assert/strict"; // Import Internal Dependencies import { parseScript, getSastAnalysis } from "../utils/index.js"; diff --git a/test/probes/isWeakCrypto.spec.js b/test/probes/isWeakCrypto.spec.js index 5727758..4343fc4 100644 --- a/test/probes/isWeakCrypto.spec.js +++ b/test/probes/isWeakCrypto.spec.js @@ -1,7 +1,7 @@ // Import Node.js Dependencies import { readFileSync, promises as fs } from "node:fs"; import { test } from "node:test"; -import assert from "node:assert/strict"; +import assert from "node:assert"; // Import Internal Dependencies import { AstAnalyser } from "../../index.js"; From f2284ddef38899c72489e2d817f1dc0fe6324ea6 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Wed, 18 Jun 2025 09:20:22 +0200 Subject: [PATCH 20/26] feat(probes): add spawnSync and execSync to unsafe-command Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 6 +- test/probes/isUnsafeCommand.spec.js | 120 ++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 3eb927f..5851f60 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -30,7 +30,7 @@ function validateNode(node) { // exec(...); if (node.type === "CallExpression" && node.callee.type === "Identifier" && - (node.callee.name === "spawn" || node.callee.name === "exec") + (node.callee.name === "spawn" || node.callee.name === "exec" || node.callee.name === "spawnSync" || node.callee.name === "execSync") ) { return [true, node.callee.name]; } @@ -40,7 +40,7 @@ function validateNode(node) { if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && - (node.callee.property.name === "spawn" || node.callee.property.name === "exec") + (node.callee.property.name === "spawn" || node.callee.property.name === "exec" || node.callee.property.name === "spawnSync" || node.callee.property.name === "execSync") ) { // child_process.spawn(...) // child_process.exec(...) @@ -80,7 +80,7 @@ function main(node, options) { // Spwaned command arguments are filled into an Array // as second arguments. This is why we should add them // manually to the command string. - if (options.data === "spawn") { + if (options.data === "spawn" || options.data === "spawnSync") { const args = node.arguments.at(1); if (args && Array.isArray(args.elements)) { args.elements.forEach((element) => { diff --git a/test/probes/isUnsafeCommand.spec.js b/test/probes/isUnsafeCommand.spec.js index fca906c..dcded59 100644 --- a/test/probes/isUnsafeCommand.spec.js +++ b/test/probes/isUnsafeCommand.spec.js @@ -69,6 +69,66 @@ test("should not detect non suspicious spawned command", () => { assert.equal(sastAnalysis.warnings().length, 0); }); +// Spawn sync + +test("should detect csrutil spawnSync command", () => { + const str = ` + const { spawnSync } = require("child_process"); + spawnSync("csrutil", ["status"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +// TODO: de-skip when the tracer would be ready +test.skip("should detect hidden csrutil spawnSync command", () => { + const str = ` + const { spawnSync: hide } = require("child_process"); + hide("csrutil", ["status"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +test("should detect csrutil spawnSync command with require", () => { + const str = ` + require("child_process").spawnSync("csrutil", ["disable"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil disable"); +}); + +test("should not detect non suspicious spawnSync command", () => { + const str = ` + const { spawnSync } = require("child_process"); + spawn("ls", ["-la"]); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + assert.equal(sastAnalysis.warnings().length, 0); +}); + // Exec test("should detect csrutil exec command", () => { @@ -128,3 +188,63 @@ test("should not detect non suspicious exec-ed command", () => { assert.equal(sastAnalysis.warnings().length, 0); }); + +// Exec sync + +test("should detect csrutil execSync command", () => { + const str = ` + const { execSync } = require("child_process"); + exec("csrutil status"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +// TODO: de-skip when the tracer would be ready +test.skip("should detect hidden csrutil execSync command", () => { + const str = ` + const { execSync: hide } = require("child_process"); + exec("csrutil status"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil status"); +}); + +test("should detect csrutil execSync command with require", () => { + const str = ` + require("child_process").execSync("csrutil disable"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + const result = sastAnalysis.getWarning(kWarningUnsafeCommand); + assert.equal(result.kind, kWarningUnsafeCommand); + assert.equal(result.value, "csrutil disable"); +}); + +test("should not detect non suspicious execSync-ed command", () => { + const str = ` + const { execSync } = require("child_process"); + exec("ls -la"); + `; + + const ast = parseScript(str); + const sastAnalysis = getSastAnalysis(str, isUnsafeCommand) + .execute(ast.body); + + assert.equal(sastAnalysis.warnings().length, 0); +}); From 6312aed4a33e8dd3b604767098eb8886b06da38d Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Wed, 18 Jun 2025 09:30:12 +0200 Subject: [PATCH 21/26] refactor(probes): add helper to detect func name Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 5851f60..f235b67 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -8,6 +8,13 @@ function isUnsafeCommand(command) { return kUnsafeCommands.some((unsafeCommand) => command.includes(unsafeCommand)); } +function isSpwanOrExec(name) { + return name === "spawn" || + name === "exec" || + name === "spawnSync" || + name === "execSync"; +} + /** * @description Detect spawn or exec unsafe commands * @example @@ -30,7 +37,7 @@ function validateNode(node) { // exec(...); if (node.type === "CallExpression" && node.callee.type === "Identifier" && - (node.callee.name === "spawn" || node.callee.name === "exec" || node.callee.name === "spawnSync" || node.callee.name === "execSync") + isSpwanOrExec(node.callee.name) ) { return [true, node.callee.name]; } @@ -40,7 +47,7 @@ function validateNode(node) { if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && - (node.callee.property.name === "spawn" || node.callee.property.name === "exec" || node.callee.property.name === "spawnSync" || node.callee.property.name === "execSync") + isSpwanOrExec(node.callee.property.name) ) { // child_process.spawn(...) // child_process.exec(...) From 1eb157c8e48373d98171ef90fc1a72266e577b94 Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Wed, 18 Jun 2025 09:31:41 +0200 Subject: [PATCH 22/26] docs(probes): update for sync Signed-off-by: Tony Gorez --- docs/unsafe-command.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/unsafe-command.md b/docs/unsafe-command.md index ea20a0a..73ba5f2 100644 --- a/docs/unsafe-command.md +++ b/docs/unsafe-command.md @@ -8,7 +8,8 @@ This warning identifies potentially dangerous use of the `spawn()` or `exec()` function from the `child_process` module. Spawning system-level commands can introduce security risks, especially if user-controlled input is involved or if the -command itself is sensitive (e.g., tools that query or change system configurations). +command itself is sensitive (e.g., tools that query or change system configurations). This warning identifies also +commands passed to `spawnSync()` and `execSync()`. > [!NOTE] > This rule is experimental. The list of suspicious commands is not exhaustive and will evolve over time. From 38224e2ef387a8c9a4aec1da56cb67a970f448fc Mon Sep 17 00:00:00 2001 From: Tony Gorez Date: Wed, 18 Jun 2025 09:32:51 +0200 Subject: [PATCH 23/26] fix: lint Signed-off-by: Tony Gorez --- src/probes/isUnsafeCommand.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index f235b67..0df79ec 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -9,10 +9,10 @@ function isUnsafeCommand(command) { } function isSpwanOrExec(name) { - return name === "spawn" || - name === "exec" || - name === "spawnSync" || - name === "execSync"; + return name === "spawn" || + name === "exec" || + name === "spawnSync" || + name === "execSync"; } /** @@ -91,8 +91,8 @@ function main(node, options) { const args = node.arguments.at(1); if (args && Array.isArray(args.elements)) { args.elements.forEach((element) => { - command += ` ${element.value}` - }) + command += ` ${element.value}`; + }); } } From 79657a84e53d493d1952977097e8d1a388960c41 Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Fri, 20 Jun 2025 22:39:08 +0200 Subject: [PATCH 24/26] Update src/probes/isUnsafeCommand.js --- src/probes/isUnsafeCommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 0df79ec..8bf7ab7 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -8,7 +8,7 @@ function isUnsafeCommand(command) { return kUnsafeCommands.some((unsafeCommand) => command.includes(unsafeCommand)); } -function isSpwanOrExec(name) { +function isSpawnOrExec(name) { return name === "spawn" || name === "exec" || name === "spawnSync" || From b87f7852d456f9e5ed3a97624496d7f35c44a222 Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Fri, 20 Jun 2025 22:39:15 +0200 Subject: [PATCH 25/26] Update src/probes/isUnsafeCommand.js --- src/probes/isUnsafeCommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 8bf7ab7..535a555 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -84,7 +84,7 @@ function main(node, options) { let command = commandArg.value; if (typeof command === "string" && isUnsafeCommand(command)) { - // Spwaned command arguments are filled into an Array + // Spawned command arguments are filled into an Array // as second arguments. This is why we should add them // manually to the command string. if (options.data === "spawn" || options.data === "spawnSync") { From b1aabe1ed4821efc5903164e9946c2afa42830bd Mon Sep 17 00:00:00 2001 From: fraxken Date: Fri, 20 Jun 2025 22:41:30 +0200 Subject: [PATCH 26/26] chore: isSpawnOrExec typo --- src/probes/isUnsafeCommand.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/probes/isUnsafeCommand.js b/src/probes/isUnsafeCommand.js index 535a555..ca12774 100644 --- a/src/probes/isUnsafeCommand.js +++ b/src/probes/isUnsafeCommand.js @@ -37,7 +37,7 @@ function validateNode(node) { // exec(...); if (node.type === "CallExpression" && node.callee.type === "Identifier" && - isSpwanOrExec(node.callee.name) + isSpawnOrExec(node.callee.name) ) { return [true, node.callee.name]; } @@ -47,7 +47,7 @@ function validateNode(node) { if ( node.callee.type === "MemberExpression" && node.callee.property.type === "Identifier" && - isSpwanOrExec(node.callee.property.name) + isSpawnOrExec(node.callee.property.name) ) { // child_process.spawn(...) // child_process.exec(...)