Skip to content

CWE-88: Improper Neutralization of Argument Delimiters in a Command ('Argument Injection') - Python

Overview

Using subprocess with an argument list instead of shell=True closes command injection: the arguments never reach a shell, so ;, | and backticks are just characters. It does not close argument injection. The arguments still reach the program, and a value that begins with - is read as an option rather than as data.

Findings on this CWE in Python almost always look correct at the call site - a list, no shell, no string formatting - and are still exploitable because a user-supplied value lands in a position where the program accepts flags.

Common Vulnerable Patterns

User input as a filename argument

import subprocess

@app.route('/create-archive')
def create_archive():
    filename = request.args.get('file')

    # VULNERABLE - argument list prevents command injection, not argument injection
    subprocess.run(['tar', '-cf', 'archive.tar', 'manifest.json', filename])

    # Attack: file=--use-compress-program=touch /tmp/pwned
    # Result: tar runs the named program in place of a compressor
    return "Archive created"

Why this is vulnerable: tar reads --use-compress-program= as the program to filter the archive through, and runs it. The value arrives as one list element, so no shell is involved and no metacharacter is needed - the program itself provides the execution primitive. Any tool with a "run this command" option (tar, rsync, zip, ssh, find) has an equivalent.

Two details decide whether the payload fires from a single injected element, and both are easy to get wrong (measured on GNU tar 1.35):

  • --use-compress-program conflicts with -z, so it works against -cf and fails against -czf with "Conflicting compression options".
  • manifest.json matters. With filename as the only non-option argument, tar answers "Cowardly refusing to create an empty archive" and exits before the option takes effect. The widely quoted --checkpoint-action=exec=... payload has the same problem twice over: it is inert without a companion --checkpoint=N, which a single element cannot supply.
import subprocess

@app.route('/search')
def search_files():
    directory = request.args.get('dir')

    # VULNERABLE - find reads a leading dash as a predicate, not a path
    result = subprocess.run(
        ['find', directory, '-name', '*.log'],
        capture_output=True
    )

    # Attack: dir=-delete
    # Result: find deletes everything below the working directory
    return result.stdout

Why this is vulnerable: find builds an expression from its arguments, so a value in a path position that starts with - becomes another predicate in that expression instead. This case is worse than most because find has no working end-of-options marker: the findutils manual notes that -- could theoretically serve as one but does not, because find ends its list of starting points at the first argument beginning with -. Unlike tar or curl, you cannot fix it by adding -- - prefix the path with ./, pass an absolute path, or use -files0-from.

Note which argument is the sink. Writing the same handler as ['find', '/var/data', '-name', pattern] puts the user value in -name's value slot, where find reads it as a literal glob and nothing happens - measured on findutils 4.10.0, -name -delete matched no files and deleted none, while -delete in the path position removed the tree. Both spellings look equally alarming in a scan result and only one of them is this weakness.

Secure Patterns

Do the work in Python instead of shelling out

import tarfile
from pathlib import Path

BASE_DIR = Path('/var/app/uploads').resolve()

# SECURE - no argv, so no flags to inject
def create_archive(filename: str) -> Path:
    source = (BASE_DIR / filename).resolve()
    if not source.is_relative_to(BASE_DIR) or not source.is_file():
        raise ValueError("Unknown file")

    archive = BASE_DIR / 'archive.tar.gz'
    with tarfile.open(archive, 'w:gz') as tar:
        tar.add(source, arcname=source.name)
    return archive


# SECURE - pathlib.glob replaces the find subprocess entirely
def search_files(pattern: str) -> list[Path]:
    if len(pattern) > 100:
        raise ValueError("Pattern too long")
    return [
        p for p in BASE_DIR.glob(pattern)
        if p.is_file() and p.resolve().is_relative_to(BASE_DIR)
    ]

Why this works: tarfile and pathlib take a path or a pattern as a typed argument, not as a position in an argument vector, so there is no parser that can reinterpret a leading - as an option. Path.resolve() followed by is_relative_to also settles path traversal, which the subprocess version left open. This is the fix that removes the weakness rather than constraining it - prefer it whenever the standard library covers the job.

Path.glob is safe against this weakness but is not a general-purpose sandbox, and the containment check in search_files is not decoration. A pattern may contain ..: measured on Python 3.13.12, BASE_DIR.glob('../secret/*') returns files outside BASE_DIR without raising, so the glob has to be filtered by where its results resolve to rather than trusted because it started from a safe root. A pattern of **/* also walks the whole subtree, so keep the length limit and consider fnmatch against a pre-listed directory when the pattern is fully untrusted.

When an external tool is genuinely required

import re
import subprocess
from pathlib import Path

BASE_DIR = Path('/var/app/uploads').resolve()
SAFE_NAME = re.compile(r'\A[A-Za-z0-9][A-Za-z0-9_.-]{0,254}\Z')

# SECURE - value can never be read as an option
def convert(filename: str) -> None:
    if not SAFE_NAME.match(filename):
        raise ValueError("Invalid filename")

    path = (BASE_DIR / filename).resolve()
    if not path.is_relative_to(BASE_DIR) or not path.is_file():
        raise ValueError("Unknown file")

    subprocess.run(
        # -y, or ffmpeg declines to overwrite and still exits 0 - see below
        ['ffmpeg', '-y', '-i', str(path), '--', str(BASE_DIR / 'out.mp4')],
        check=True,
        timeout=60,
    )

Why this works: Three independent things have to hold, and each covers a different failure:

  • The pattern requires the first character to be alphanumeric, so the value cannot start with - at all. This is the check that actually stops flag injection, and it is stated as "must start with something safe" rather than "must not start with a dash" - a denylist of prefixes misses --, unicode dashes, and whitespace-prefixed values.
  • Resolving under a fixed base directory means a name that passes the pattern still cannot escape the directory.
  • -- tells tools that support it to stop parsing options, which covers the case where validation is later loosened. It is defense in depth, not the primary control: find ignores it, and it does nothing for a value in an option's value position. ffmpeg does honour it - verified on 8.0.1, an output named -weird.mp4 is written rather than parsed.

-y is not part of the security fix and is here because leaving it out breaks the example: measured on ffmpeg 8.0.1, an existing output file makes ffmpeg print "Not overwriting - exiting" and exit 0, so check=True reports success for a conversion that never happened.

Considerations

  • Whether the argument is attacker-positioned at all. A value in a bare positional slot (['grep', pattern, path]) is the exploitable case: a leading - makes it an option. A value that an option consumes is not - ['grep', '-e', pattern] hands pattern to -e as the pattern to match, so -e --help searches for the literal string --help rather than printing usage, measured on GNU grep 3.0. The same holds for a value concatenated onto a fixed prefix, ['grep', f'--include={pattern}'], which is one argument beginning with --include= whatever the value starts with. Trace where the string lands in the list before deciding the finding is real - this is the same distinction as the find example above, and it is what separates a finding from a false positive rather than what separates severities.
  • Which tool is on the other end. The severity is a property of the program, not of Python. tar, find, curl, git, ssh and rsync all have options that read or write arbitrary paths or execute commands; GTFOBins is the practical reference for what a given binary gives away. A tool with no such option is a much weaker finding.
  • How the tool spells an option's value. One list element is one argv entry, so only an option that carries its own value - --opt=value, or an attached short option such as -K/path - can be supplied by a single injection. ffmpeg is the instructive counter-example: it has no --opt=value form at all, so -f data arrives as one unrecognised option and every dangerous option needs a second element the attacker does not have. Measured on 8.0.1, a single injected element there is a failed conversion rather than a file write, which is a defensible false positive - record it with that reasoning rather than as "arrays are safe".
  • shlex.quote() does not apply here. It is for building shell strings, and the safe form of this code has no shell. Quoting an argument that is already a list element adds literal quote characters to the filename and fixes nothing.

Testing

Re-running the scanner cannot confirm this fix: the call site looked correct before the change and still does afterwards. The assertions are behavioural.

  • Send --use-compress-program=touch /tmp/pwned (or the equivalent option for your tool) and assert the request is rejected and /tmp/pwned does not exist. A 400 alone does not prove the process never ran.
  • Check the payload against the unfixed code first. A payload that cannot fire passes this assertion whether or not the fix works, which is how the usual --checkpoint-action=exec=... string turns a real finding into an apparently clean test - it needs a companion --checkpoint=N that the injection point cannot supply.
  • Send a leading-dash value with a valid body (-rf, --help) and assert rejection. --help is useful because a tool that accepts it usually exits 0, which makes an accidental pass look like success in the logs.
  • Send legitimate names that the pattern must still accept - a leading digit, an internal dot, a hyphen that is not first (2024-report.v2.tar). Tightening this validation is the most common way the fix breaks real users.
  • Where you replaced the subprocess with tarfile or pathlib, assert the output is byte-identical to what the tool produced for a known input, so the migration is not silently lossy.

Additional Resources