Skip to content

Fix command injection in Custom Command#1363

Open
codingfrog wants to merge 1 commit into
arsenetar:masterfrom
codingfrog:security/fix-command-injection
Open

Fix command injection in Custom Command#1363
codingfrog wants to merge 1 commit into
arsenetar:masterfrom
codingfrog:security/fix-command-injection

Conversation

@codingfrog
Copy link
Copy Markdown

@codingfrog codingfrog commented Feb 14, 2026

Context

During a security audit of dupeGuru, we identified a command injection vulnerability in the Custom Command feature.

Vulnerability

The code at core/app.py in invoke_custom_command() constructs a shell command by performing string substitution on the user's custom command template, replacing %d and %r with file paths, then passes the resulting string to subprocess.Popen() with shell=True:

cmd = cmd.replace("%d", str(dupe.path))
cmd = cmd.replace("%r", str(ref.path))
p = subprocess.Popen(cmd, shell=True, ...)

Because shell=True passes the entire string to the system shell for interpretation, any shell metacharacters in file paths are evaluated. A file named $(rm -rf ~).txt would cause arbitrary command execution when the user invokes a custom command on it.

The existing Windows workaround (wrapping paths in quotes) is insufficient — it doesn't handle all injection vectors and doesn't protect Unix/macOS at all.

Attack scenario

  1. An attacker places a file with a crafted name in a directory the user scans
  2. dupeGuru detects it as a duplicate
  3. The user invokes their custom command (e.g., a diff tool) on this file
  4. The shell executes the embedded command with the user's privileges

Fix

  • Use shlex.split(cmd) to tokenize the command template into a list of arguments
  • Perform %d/%r substitution on each token individually (so paths stay within their token boundary)
  • Call subprocess.Popen(cmd_parts, shell=False) — arguments are passed directly to the OS without shell interpretation
  • Remove the Windows quoted-path workaround (no longer needed with shell=False)
  • Add proper error handling for malformed command templates and execution failures

Test plan

  • pytest core/tests/app_test.py — 38 tests pass
  • Manual test: create a file with shell metacharacters in the name, invoke custom command, verify no shell expansion occurs

🤖 Generated with Claude Code

Replace shell=True subprocess calls with properly tokenized
arguments using shlex.split() and shell=False. This prevents
command injection when file paths contain shell metacharacters.

- Use shlex.split() to tokenize custom command string
- Replace %d/%r placeholders in tokenized arguments
- Execute with shell=False for security
- Add error handling with user feedback
- Remove Windows quoted-path workaround (unnecessary with shell=False)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants