# Root Cause Analysis: Node.js Task Runner Single-Quote Escape Bug

## Summary

Node.js' task runner (`node --run <task> -- <args>`) uses an `EscapeShell()` routine in `src/node_task_runner.cc` to safely quote positional arguments before concatenating them into a `/bin/sh -c` command string. On POSIX systems, the vulnerable code escaped each single quote (`'`) by rewriting it to `\'` and wrapping the result in single quotes. Because a backslash is **literal** inside POSIX single-quoted strings, the rewritten `'` actually **terminates** the single-quoted context early, leaving the remainder of the argument to be interpreted as raw shell syntax. An attacker-controlled argument containing a single quote followed by shell metacharacters can break out and inject arbitrary commands. The fix (commit `e76c573e4546ce9e89e0dd954f80aaba32148a48`) replaces `\'` with the POSIX-safe sequence `'"'"'` (close single-quote, double-quote the literal quote, reopen single-quote).

## Impact

- **Package/component affected:** Node.js core — `src/node_task_runner.cc`, the `EscapeShell()` function used by `node --run`.
- **Affected versions:** All Node.js builds that include the task runner (`--run` flag) and are prior to commit `e76c573e4546ce9e89e0dd954f80aaba32148a48`. Verified vulnerable on system Node.js v24.18.0.
- **Risk level and consequences:** High. Any application that forwards user-controlled data as arguments to `node --run` (e.g., a CI runner, a build tool, or a web backend that shells out to `node --run`) is vulnerable to **command injection** — arbitrary command execution with the privileges of the Node.js process. Even without metacharacters, a benign single quote in an argument (e.g., a person's name like "I'm") causes a `/bin/sh` syntax error, breaking the script entirely (denial of service).

## Impact Parity

- **Disclosed/claimed maximum impact:** Command injection / code execution via `node --run` arguments on Unix-like systems.
- **Reproduced impact from this run:** **Full command injection confirmed.** An argument `x';id > MARKER;echo INJECTION_PROVEN #` causes the `id` command to execute (writing its output to a marker file) and `echo INJECTION_PROVEN` to print to stdout — arbitrary attacker-controlled commands run via `/bin/sh -c`. Additionally, the benign argument `"I think therefore I'm"` causes a `/bin/sh` syntax error (DoS / broken argument passing).
- **Parity:** `full` — the claimed code-execution impact was demonstrated through the real `node --run` CLI entrypoint.
- **Not demonstrated:** N/A — code execution was achieved.

## Root Cause

In POSIX shells, single-quoted strings preserve **every** character literally — a backslash has no special meaning inside single quotes. The vulnerable `EscapeShell()` code:

```cpp
std::string escaped =
    std::regex_replace(std::string(input), std::regex("'"), "\\'");
escaped = "'" + escaped + "'";
```

rewrites each `'` as `\'` and wraps in `'...'`. For input `x';id > M #`:
- After replace: `x\';id > M #`
- After wrap: `'x\';id > M #'`

`/bin/sh -c` parses `'x\'` as a single-quoted string containing `x\` (the backslash is literal). The `'` after `\` **closes** the single-quote context. The remainder `;id > M #` is now **unquoted shell syntax**: `;` is a command separator, `id > M` executes `id` with output redirected to `M`, and `#` begins a comment that consumes the trailing wrap-quote `'` — avoiding any syntax error.

The correct POSIX-safe technique is `'"'"'` (close the single quote, insert a literal `'` via double quotes, reopen the single quote). The fix commit applies this:

```cpp
std::regex_replace(std::string(input), std::regex("'"), "'\"'\"'");
```

With the fix, the same input becomes `'x'"'"';id > M #'`, which `/bin/sh` parses as a single literal argument `x';id > M #` with no shell interpretation.

**Fix commit:** `e76c573e4546ce9e89e0dd954f80aaba32148a48` ("src: fix escaping of single quotes in task runner", PR #64089, ref. HackerOne #3817602).

## Reproduction Steps

1. **Script:** `bundle/repro/reproduction_steps.sh`
2. **What the script does:**
   - Reads `bundle/project_cache_context.json` to locate the prepared Node.js source cache.
   - Builds (or reuses pre-built) Node.js binaries at the **vulnerable** commit (`e76c573^`) and the **fixed** commit (`e76c573`).
   - Creates a test project with `package.json` containing an npm script `showargs` mapped to `echo`.
   - Runs **two attempts** per test for both builds:
     - **Test A (DoS):** `node --run showargs -- "I think therefore I'm"` — vulnerable build produces `/bin/sh: Syntax error: Unterminated quoted string`; fixed build prints `I think therefore I'm` cleanly.
     - **Test B (Command injection):** `node --run showargs -- "x';id > MARKER;echo INJECTION_PROVEN #"` — vulnerable build creates a marker file containing `id` output and prints `INJECTION_PROVEN`; fixed build passes the argument literally with no marker file.
     - **Test C (Ticket's exact payload):** `node --run showargs -- "foo' ; id ; '"` — vulnerable build produces a syntax error (DoS only, since the trailing unbalanced quote is not commented out); fixed build passes the argument safely.
   - Writes `bundle/repro/runtime_manifest.json` with runtime evidence.
3. **Expected evidence of reproduction:**
   - Marker file (`marker_vuln_attempt*.txt`) containing `uid=...` output from the injected `id` command.
   - Test logs showing `INJECTION_PROVEN` in stdout for the vulnerable build.
   - Test logs showing `/bin/sh: Syntax error: Unterminated quoted string` for DoS tests.
   - Absence of marker file and clean literal output for the fixed build (negative control).

## Evidence

- **Log file locations:**
  - `bundle/logs/test_A_vuln_attempt{1,2}.log` — DoS test on vulnerable build
  - `bundle/logs/test_A_fixed_attempt{1,2}.log` — DoS test on fixed build
  - `bundle/logs/test_B_vuln_attempt{1,2}.log` — Injection test on vulnerable build
  - `bundle/logs/test_B_fixed_attempt{1,2}.log` — Injection test on fixed build
  - `bundle/logs/test_C_vuln_attempt{1,2}.log` — Ticket payload on vulnerable build
  - `bundle/logs/test_C_fixed_attempt{1,2}.log` — Ticket payload on fixed build
  - `bundle/logs/binary_info.txt` — Node.js binary versions/paths
  - `bundle/logs/source_diff.txt` — Vulnerable vs. fixed EscapeShell source
  - `bundle/repro/artifacts/marker_vuln_attempt*.txt` — Proof of injected `id` execution
- **Key excerpts (pre-verified on system Node.js v24.18.0):**
  - Injection: stdout shows `["x\\"]` then `INJECTION_PROVEN`; marker file contains `uid=1000(vscode) gid=1000(vscode) groups=1000(vscode),969(969)`
  - DoS: `/bin/sh: 1: Syntax error: Unterminated quoted string`
- **Environment:** Linux x86_64, `/bin/sh` (dash), Node.js built from source at commits `e76c573^` (vulnerable) and `e76c573` (fixed). System Node.js v24.18.0 also confirmed vulnerable.

## Recommendations / Next Steps

- **Suggested fix approach:** Apply commit `e76c573e4546ce9e89e0dd954f80aaba32148a48` — replace `\\'` with `'\"'\"'` in the POSIX branch of `EscapeShell()`. This is the canonical POSIX-safe single-quote escaping technique.
- **Upgrade guidance:** Upgrade to any Node.js release that includes commit `e76c573`. All versions prior to this commit that ship the `--run` task runner are affected.
- **Testing recommendations:** Add regression tests that pass arguments containing single quotes, semicolons, and comment characters through `node --run` and verify the shell receives them as literal values. The existing `test/cctest/test_node_task_runner.cc` `EscapeShell` unit test should be expanded to cover the `'"'"'` output and end-to-end `--run` invocations with shell metacharacters.

## Additional Notes

- **Idempotency:** The script cleans up marker files before each injection test and recreates the test project from scratch, so repeated runs produce consistent results.
- **Key insight:** The ticket's exact payload `foo' ; id ; '` only causes a syntax error (DoS) because the trailing wrap-quote `'` opens an unterminated single-quoted context, causing `/bin/sh` to reject the entire command before executing anything. The modified payload `x';id > MARKER;echo INJECTION_PROVEN #` uses a `#` comment to consume the trailing wrap-quote, producing a **syntactically valid** shell command in which the injected `id` and `echo` commands execute. This achieves the full code-execution impact claimed by the ticket.
- **Why the `#` technique works:** After the broken `\'` escaping closes the single-quote context, the `#` (preceded by whitespace) starts a shell comment that extends to end-of-line, swallowing the wrap's trailing `'`. The commands before `#` (`id`, `echo`) execute normally.
- **Platform scope:** This vulnerability is POSIX-only. The Windows branch of `EscapeShell()` uses a different escaping strategy (double-quote based) and is not affected.