How command injection happens in JavaScript/Node.js and how to fix it

Introduction

The scripts/build.js file in this Node.js library handles generating SVG images that visualize CSV data changes across git commits, but a flaw in the diffSVG() function created a security risk that Semgrep flagged as a high-severity command injection primitive. Specifically, the function at line 337 took the stdout output from a git log command — commit hashes — and fed it directly into a second shell command via child_process.exec(), without any validation.

While this particular script isn't directly exposed to remote attackers today, it's exactly the kind of pattern that automated exploit tooling looks for: a function argument flowing, unsanitized, into a shell-executing API. If the surrounding code ever changes — for example, if baseFolder, folderSrc, or fileNameSrc become configurable from a config file, CLI flag, or CI environment variable — this becomes a directly exploitable command injection vector.

The Vulnerability Explained

Here's the vulnerable code before the fix:

import { exec } from 'child_process';

function diffSVG() {
  // Get the last two commits that modified the CSV file
  exec(
    `git log -n 2 --pretty=format:"%H" -- ${baseFolder}${folderSrc}${fileNameSrc}.csv`,
    function (err, stdout, stderr) {
      // ...
      const newerCommit = commits[0];
      const olderCommit = commits[1];

      // Compare the two commits
      exec(
        `git diff -w -U0 ${olderCommit} ${newerCommit} -- ${baseFolder}${folderSrc}${fileNameSrc}.csv`,
        function (err, stdout, stderr) {
          // ...
        }
      );
    }
  );
}

There are two distinct problems here:

1. Untrusted values interpolated into a shell string. exec() spawns a full shell (/bin/sh -c "...") and passes the entire string to it. Any shell metacharacters (;, &&, |, `, $()) embedded in baseFolder, folderSrc, fileNameSrc, newerCommit, or olderCommit would be interpreted by the shell, not treated as literal arguments.
2. Chained trust. The second exec() call trusts newerCommit and olderCommit — values parsed from the output of the first command — without ever checking their format. Anywhere upstream that this data could be influenced (a crafted git repo, a malicious commit message format, a compromised .git directory, or future refactors that let these values come from user input) turns this into a working injection.

Example attack scenario: Imagine this build script is later adapted to accept a --file CLI argument to build diffs for arbitrary CSV files (a very plausible evolution of a build tool). If an attacker controls that filename value — say report.csv; rm -rf ~ #.csv — the interpolated string passed to exec() would execute the injected command with the same privileges as the build process. Because exec() invokes a shell, there's no boundary preventing this.

The Fix

The PR makes two complementary changes to scripts/build.js:

1. Replaced exec() with execFile():

// Before
import { exec } from 'child_process';
exec(
  `git log -n 2 --pretty=format:"%H" -- ${baseFolder}${folderSrc}${fileNameSrc}.csv`,
  function (err, stdout, stderr) { ... }
);

// After
import { execFile } from 'child_process';
execFile(
  'git',
  ['log', '-n', '2', '--pretty=format:%H', '--', `${baseFolder}${folderSrc}${fileNameSrc}.csv`],
  function (err, stdout, stderr) { ... }
);

execFile() does not spawn a shell by default — it invokes the git binary directly with an argument array. Each array element is passed as a discrete argument, so shell metacharacters in baseFolder/folderSrc/fileNameSrc are no longer interpreted; they're treated as literal, inert strings passed to git.

2. Added regex validation for commit hashes before the second execFile() call:

// Validate commit hashes to prevent command injection
const shaRegex = /^[0-9a-f]{40}$/;
if (!shaRegex.test(newerCommit) || !shaRegex.test(olderCommit)) {
  console.error('Invalid commit hash format');
  return;
}

// Compare the two commits
execFile(
  'git',
  ['diff', '-w', '-U0', olderCommit, newerCommit, '--', `${baseFolder}${folderSrc}${fileNameSrc}.csv`],
  function (err, stdout, stderr) { ... }
);

This is defense in depth: even though execFile() already eliminates shell interpretation, the regex ensures newerCommit and olderCommit are strictly 40-character lowercase hex strings (the exact format of a git SHA-1 hash) before they're used at all. Anything that doesn't match — malformed output, injected metacharacters, or unexpected git output formatting — causes the function to bail out safely with a logged error instead of proceeding.

Together, these two changes close both layers of the risk: the shell-execution surface (exec → execFile) and the untrusted-data surface (unchecked stdout parsing → regex-validated hashes).

Prevention & Best Practices

• Never use child_process.exec() with interpolated strings. Prefer execFile() or spawn(), which accept an argument array and don't invoke a shell.
• Validate data even when it "should" be safe. Git commit hashes are always 40 hex characters — encode that assumption explicitly with a regex rather than trusting parsed stdout.
• Treat any output from an external process as untrusted input to the next process call, especially in build/CI scripts that often run with elevated permissions.
• Run static analysis in CI. Semgrep's detect-child-process rule catches exactly this pattern — a child_process call built from a function argument.
• Follow OWASP guidance on OS command injection prevention, including allow-listing input formats and avoiding shell invocation entirely where possible.

Key Takeaways

• scripts/build.js's diffSVG() function chained two exec() calls, where the second trusted unvalidated output from the first — a classic taint-propagation pattern.
• Switching from exec() to execFile() removes the shell-interpretation risk by passing arguments as an array instead of a concatenated string.
• The added shaRegex = /^[0-9a-f]{40}$/ check enforces that only well-formed git SHA-1 hashes reach the second execFile() call.
• Even "internal" build tooling should be hardened — a future refactor that exposes baseFolder/fileNameSrc to user input would have made this immediately exploitable.
• Semgrep's detect-child-process rule is effective at catching child_process calls fed by function arguments before they become real vulnerabilities.

How Orbis AppSec Detected This

• Source: stdout from the first git log exec() call (commit hashes), and the file/path-building variables (baseFolder, folderSrc, fileNameSrc) used to construct shell command strings
• Sink: exec() call in diffSVG() at scripts/build.js:337, which passed a template-literal string containing ${olderCommit} ${newerCommit} directly to a shell
• Missing control: No validation of commit hash format, and no separation between command arguments and the shell command string
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection'))
• Fix: Replaced exec() with execFile() using an argument array, and added a /^[0-9a-f]{40}$/ regex check to validate commit hashes before use

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This fix demonstrates a core principle of secure coding: shell-executing APIs and untrusted (or even semi-trusted) data don't mix, no matter how "internal" a script feels. By replacing child_process.exec() with execFile() and validating commit hashes against a strict regex, scripts/build.js eliminated a command injection primitive before it could be chained into a real exploit by future code changes or automated tooling. Auditing build scripts and CI tooling for these patterns is just as important as auditing application code — build systems often run with broad filesystem and network access, making them a high-value target.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP OS Command Injection Defense Cheat Sheet
• Node.js child_process.execFile() documentation
• Semgrep rule: detect-child-process
• harden: sanitize child_process call in build.js...