Skip to content

Switch executeCommand from shell exec to argv-based execFile (CWE-78)#164

Open
JAE0Y2N wants to merge 1 commit into
tiged:mainfrom
JAE0Y2N:cwe78-execfile-fix
Open

Switch executeCommand from shell exec to argv-based execFile (CWE-78)#164
JAE0Y2N wants to merge 1 commit into
tiged:mainfrom
JAE0Y2N:cwe78-execfile-fix

Conversation

@JAE0Y2N

@JAE0Y2N JAE0Y2N commented May 21, 2026

Copy link
Copy Markdown

What

src/utils.ts defines executeCommand as promisify(child_process.exec). The shell form spawns /bin/sh -c <command> and parses shell metacharacters in interpolated arguments. Three call sites pass user-controlled repo.url, repo.ref, and url (sourced from the slug, e.g. user/repo#refs/...) into template strings:

  • src/utils.ts:639git ls-remote ${repo.url} ${repo.ref}
  • src/utils.ts:806git fetch --depth 1 ${repo.url} ${ref}
  • src/tiged.ts:1095-1105 — three compound branches:
    • Windows: cd ${dest} && git init && git remote add origin ${url} && git fetch --depth 1 origin ${ref} && git checkout FETCH_HEAD
    • POSIX: same shape with ; separators
    • Default: git clone --depth 1 ${url} ${dest}

A repo slug like foo/bar; touch /tmp/pwned parses under the shell as git ls-remote foo/bar + touch /tmp/pwned. CWE-78.

How this PR fixes it

executeCommand now delegates to promisify(child_process.execFile). The new signature is (cmd, args?, options?):

  • When called with explicit args, the array is passed to execFile verbatim — no shell, no metacharacter parsing.
  • When called with a single whitespace-only string (e.g., 'git --version', 'git init', 'git rev-list FETCH_HEAD'), the back-compat path splits and runs through execFile for safety. Only developer-authored constants use this path.

The five user-input call sites are rewritten to argv form. The shell-compound branches in src/tiged.ts collapse into sequential executeCommand calls with the cwd option — once you drop the shell, the Windows && vs POSIX ; separator distinction disappears.

How to verify

# Before: shell-injection lands
git ls-remote 'foo/bar; touch /tmp/pwned' anymain    # touch fires under exec
ls /tmp/pwned                                          # exists

# After (with this PR): argv-array passes the literal arg
# git rejects 'foo/bar; touch /tmp/pwned' as a bad ref, no shell expansion

The companion fix landed on Rich-Harris/degit as PR #404 (same maintainer pattern, same execFile switch).

Surface

Two files, ~50 LOC. No new dependencies. No tests added — happy to add a child_process.exec spy that asserts execFile is the only sink if you'd like.

@nake89

nake89 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Thanks. Fixed for 2.12.8. Got inspiration from your degit PR. Credit you in commit. Visible in branch: https://github.com/tiged/tiged/tree/fix/release-2.12-cwe78

Will soon look at this, to get it merged in main branch.

But most important was to fix the publicly released version. Fix has been deployed: https://www.npmjs.com/package/tiged?activeTab=versions

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants