Skip to content

JS: js/command-line-injection misses child_process argument arrays when the shell option is set #22825

Open

Description

@wildoranges

Description of the issue

js/command-line-injection does not report remote input that reaches the argument array of spawn/execFile when the shell option is set, although Node.js then runs the joined command line in a shell. Checked with the latest release, CodeQL 2.27.2 (codeql/javascript-queries 2.4.7).

const cp = require("child_process");
const app = require("express")();

app.get("/a", (req, res) => {
  cp.exec("echo hello " + req.query.name); // reported
  res.send("ok");
});

app.get("/b", (req, res) => {
  cp.spawn("echo", ["hello", req.query.name], { shell: true }); // not reported
  res.send("ok");
});

app.get("/c", (req, res) => {
  cp.spawn("echo", ["hello", req.query.name], { shell: "/bin/sh" }); // not reported
  res.send("ok");
});

Only /a is reported, by js/command-line-injection. For /b and /c, no query in javascript-security-extended.qls reports a command injection. At runtime both run /bin/sh -c "echo hello <name>" (Node.js joins [file, ...args] with spaces when shell is truthy; see the spawn documentation), so name = "x; touch /tmp/marker" creates the file (Node.js v24.18.0).

As far as I can tell, CommandInjection::SystemCommandExecutionSink only uses getACommandArgument(), which for spawn/execFile is the program name. The argument array is treated as shell-interpreted only in UnsafeShellCommandConstruction (executesArrayAsShell), i.e. for library inputs, and even there only for
shell: true: with a string-valued shell such as "/bin/sh", js/shell-command-constructed-from-input misses the call as well.

Activity

  1. vishalshetagar commented on Oct 24, 2026

    @vishalshetagar

    I’d like to work on this issue by updating the command-injection sink modeling for spawn and execFile when the shell option is enabled, including string-valued shell paths, and adding regression tests for attacker-controlled arguments.

  2. sam-robson commented on Nov 2, 2026

    @sam-robson
    Contributor

    Thanks for the detailed report @wildoranges, and @vishalshetagar for offering to help! Your contribution would be most welcome 👍🏻

  3. vishalshetagar commented on Nov 2, 2026

    @vishalshetagar

    Thanks @sam-robson ! I’ve opened #22978 to address this. It adds detection for remote input in spawn/execFile argument arrays with shell: true or a string-valued shell path, and updates the library-input query to recognize string-valued shell paths too. It also handles execFile calls without a callback.

    The targeted checks and existing query test suites pass. Would appreciate your review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions