diff --git a/javascript/ql/lib/change-notes/2026-11-02-shell-command-arguments.md b/javascript/ql/lib/change-notes/2026-11-02-shell-command-arguments.md new file mode 100644 index 000000000000..5bb8a5009a80 --- /dev/null +++ b/javascript/ql/lib/change-notes/2026-11-02-shell-command-arguments.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Command injection modeling now recognizes argument arrays passed to `spawn` and `execFile` when the `shell` option is enabled, including string-valued shell paths and calls to `execFile` without a callback. diff --git a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll index 10801d966542..39ffa3f6bf27 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll @@ -813,8 +813,8 @@ module NodeJSLib { methodName.matches("%Sync") then result = this.getLastArgument() else - // the rest (exec/execFile) has the options argument as their second last. - result = this.getParameter(this.getNumArgument() - 2).asSink() + // exec/execFile may have a callback after the options argument. + result = this.getParameter([this.getNumArgument() - 2, this.getNumArgument() - 1]).asSink() } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/CommandInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/CommandInjectionCustomizations.qll index b3516a79bf49..7db735de3889 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/CommandInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/CommandInjectionCustomizations.qll @@ -5,6 +5,7 @@ */ import javascript +private import UnsafeShellCommandConstructionCustomizations module CommandInjection { /** @@ -55,6 +56,19 @@ module CommandInjection { SystemCommandExecutionSink() { this = any(SystemCommandExecution sys).getACommandArgument() } } + /** An argument in a `child_process` argument array that is interpreted by a shell. */ + private class ChildProcessArgumentSink extends Sink { + ChildProcessArgumentSink() { + exists(UnsafeShellCommandConstruction::ShellTrueCommandExecutionSink sink | + this = sink and + sink.getCommandExecution() = + API::moduleImport(["mz/child_process", "child_process"]) + .getMember(["execFile", "execFileSync", "spawn", "spawnSync"]) + .getMaybePromisifiedCall() + ) + } + } + private class SinkFromModel extends Sink { SinkFromModel() { ModelOutput::sinkNode(this, "command-injection") } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll index e65fac9530d7..03ccd20640d2 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll @@ -175,20 +175,20 @@ module UnsafeShellCommandConstruction { } /** - * Holds if the arguments array given to `sys` is joined as a string because `shell` is set to true. + * Holds if the arguments array given to `sys` is joined as a string because the `shell` option is enabled. */ predicate executesArrayAsShell(SystemCommandExecution sys) { - sys.getOptionsArg() - .getALocalSource() - .getAPropertyWrite("shell") - .getRhs() - .asExpr() - .(BooleanLiteral) - .getValue() = "true" - or - exists(API::Node node | - node.asSink() = sys.getOptionsArg() and - node.getMember("shell").asSink().mayHaveBooleanValue(true) + exists(DataFlow::Node shell | + shell = sys.getOptionsArg().getALocalSource().getAPropertyWrite("shell").getRhs() + or + exists(API::Node node | + node.asSink() = sys.getOptionsArg() and + shell = node.getMember("shell").asSink() + ) + | + shell.mayHaveBooleanValue(true) + or + exists(string path | shell.mayHaveStringValue(path) and path != "") ) } @@ -208,13 +208,13 @@ module UnsafeShellCommandConstruction { } /** - * An argument to a command invocation where the `shell` option is set to true. + * An argument to a command invocation where the `shell` option is enabled. */ class ShellTrueCommandExecutionSink extends Sink { SystemCommandExecution sys; ShellTrueCommandExecutionSink() { - // `shell` is set to true. That means string-concatenation happens behind the scenes. + // `shell` is enabled. That means string-concatenation happens behind the scenes. // We just assume that a `shell` option in any library means the same thing as it does in NodeJS. exists(DataFlow::SourceNode arr | arr = endsInShellExecutedArray(DataFlow::TypeBackTracker::end(), sys) diff --git a/javascript/ql/src/change-notes/2026-11-02-shell-command-arguments.md b/javascript/ql/src/change-notes/2026-11-02-shell-command-arguments.md new file mode 100644 index 000000000000..9b0e590d9af7 --- /dev/null +++ b/javascript/ql/src/change-notes/2026-11-02-shell-command-arguments.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `js/command-line-injection` query now reports remote input that reaches argument arrays when the `shell` option is enabled. The `js/shell-command-constructed-from-input` query also recognizes string-valued shell paths. Both queries now handle `execFile` calls without a callback.