Fix OS command and argument injection in Python and Flask

1. Find every subprocess.run, Popen, call or check_output with shell=True, and every os.system/os.popen, whose command is built from request.args, form or JSON data.
2. Pass a list of arguments and drop shell=True (the default is shell=False): subprocess.run([tool, '--', name], capture_output=True, text=True, check=True).
3. Put a literal '--' in the list before the first user-controlled operand, so a value beginning with '-' is an operand and not an option.
4. For any argument that selects the command, a subcommand, a flag or an enumerated option value, map the caller's choice through an allowlist (a dict) and raise to return 400 for anything else.
5. Add regression tests with this card's payloads (separators, substitution, newline, and a leading-dash option) that assert a marker file was never created, next to functional tests for names with spaces, apostrophes and quotes.
6. Log the tool's stderr on the server and return a generic 500; its error text helps an attacker tune a payload.

The three implementations share one API and one test suite; only the code below changes. The tests load the one that IMPL names.

def greet(tool: str, name: str) -> str:
    # shell=True runs the string through /bin/sh: every metacharacter in `name`
    # is live, and `; | && $( ) backticks` and newlines all start new work.
    cmd = f"{tool} {name}"
    return subprocess.run(cmd, shell=True, capture_output=True, text=True, check=True).stdout

def greet(tool: str, name: str) -> str:
    # A list argument with the default shell=False runs the program directly: no
    # shell parses the arguments. "--" makes everything after it an operand, so a
    # name that starts with a dash cannot pose as an option.
    argv = [tool, "--", name]
    return subprocess.run(argv, capture_output=True, text=True, check=True).stdout

# The format picks part of the command itself, which a value can never do
# safely, so map the caller's choice onto a fixed flag value from an allowlist.
FORMAT_FLAGS = {"text": "text", "json": "json"}


def report(tool: str, name: str, fmt: str) -> str:
    flag = FORMAT_FLAGS.get(fmt)
    if flag is None:
        raise ValueError("unsupported format")
    argv = [tool, "--format", flag, "--", name]
    return subprocess.run(argv, capture_output=True, text=True, check=True).stdout

# The "obvious" shell separators. Note what is missing: $ ( ) and backticks
# (command substitution), and the leading dash of an injected option.
_BLOCKED = re.compile(r"[;|&<>\n]")


def _shell_quote(value: str) -> str:
    if _BLOCKED.search(value):
        raise ValueError("illegal character in input")
    return f'"{value}"'


def greet(tool: str, name: str) -> str:
    cmd = f"{tool} {_shell_quote(name)}"
    return subprocess.run(cmd, shell=True, capture_output=True, text=True, check=True).stdout

The exploit tests send the attack and check that it fails; functional tests next to them keep ordinary requests working, so a fix that rejects everything fails too.

# Exploit: each payload must stay data; the marker file must never be created.
# Each payload names a fresh marker path; a clean run leaves it absent.

def assert_no_marker(client, path: str, marker: Path, params: dict) -> None:
    client.get(path, query_string=params)  # the tool runs synchronously before the response
    assert not marker.exists(), f"marker was created: the payload executed ({params})"


def test_exploit_semicolon_does_not_start_a_second_command(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice; touch {m}"})


def test_exploit_pipe_does_not_start_a_second_command(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice | touch {m}"})


def test_exploit_and_chain_does_not_start_a_second_command(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice && touch {m}"})


def test_exploit_command_substitution_does_not_run(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice$(touch {m})"})


def test_exploit_backticks_do_not_run(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice`touch {m}`"})


def test_exploit_newline_does_not_start_a_second_command(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"Alice\ntouch {m}"})


def test_exploit_leading_output_option_cannot_write_a_file(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"--output={m}"})


def test_exploit_leading_short_option_cannot_write_a_file(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/greeting", m, {"name": f"-o{m}"})


def test_exploit_option_injected_through_format_is_rejected(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/report", m, {"name": "Alice Martin", "format": f"text --output={m}"})


def test_exploit_leading_dash_name_is_an_operand_not_an_option(client, tmp_path):
    m = tmp_path / "pwned"
    assert_no_marker(client, "/report", m, {"name": f"--output={m}", "format": "text"})

• shell=True is the whole bug. With a list and shell=False the elements are passed as argv and never reach a shell.
• With shell=True, subprocess takes a string; passing a list instead runs only the first element as the program and the rest become the shell's own arguments, which is a different surprise.
• subprocess with a list still runs one program, but without '--' an operand that starts with '-' is parsed as an option. Argument injection (CWE-88) needs no shell.
• shlex.quote() quotes one token for the shell, but you then still depend on the shell; prefer a list and no shell. shlex.split() is not a safety tool.
• os.system and os.popen always use the shell and have no list form; replace them with subprocess and a list.
• Flask's request.args.get returns a string or None; a repeated key needs getlist, and values from JSON may be non-strings. Validate before use.
• Allowlist by value, not by a regex on the raw input: a blocklist of shell metacharacters still lets --output= and -o through, because a dash is not a metacharacter.

1. Run the tests in all three modes: IMPL=fixed must pass; IMPL=vulnerable and IMPL=wrong must fail (node ../../run-all.mjs python-flask does this for you).
2. Against your own endpoint, send name=Alice;touch /tmp/pwned and name=--output=/tmp/pwned and confirm /tmp/pwned is never created.
3. Run semgrep --config r/python.lang.security.audit.subprocess-shell-true.subprocess-shell-true impl and bandit -r impl: the shell-true rule and Bandit B602 (HIGH) fire only on vulnerable.py and wrong.py, while Bandit adds LOW B603/B404 advisories on the fixed list form. Static analysis flags the shell, not the argument injection, so keep the regression tests.

What each rule reported when it was run on this card's code, or what its source shows it covers where it was not run. A rule that exists for the weakness does not always catch it.

Download the project, install its dependencies once (this step needs the network), then run the tests against each implementation from the command-injection/python-flaskfolder.

python3 -m venv .venv
.venv/bin/python -m pip install -r requirements.txt

# Expected: every test passes
IMPL=fixed .venv/bin/python -m pytest

# Expected: an exploit test fails
IMPL=vulnerable .venv/bin/python -m pytest

# Expected: a test fails
IMPL=wrong .venv/bin/python -m pytest

Each run writes a JUnit report to reports/junit.xml. From the top folder of the download, node run-all.mjs command-injection/python-flask runs all three modes and checks the expected results.

Download the project ZIP, 17 KB

Verified

Machine

macOS 27.0 (Darwin 27.0.0, arm64)

Toolchain

• python 3.14.8
• flask 3.1.3
• pytest 9.1.1
• shell /bin/sh (dash-compatible POSIX shell)

Matrix

fixed: 16 of 16 tests pass; vulnerable: 16 of 16 tests fail (10 exploit tests); wrong: 7 of 16 tests fail (5 exploit tests)

Dependency freshness

flask 3.1.3 (2026-02-19) and pytest 9.1.1 (2026-06-19) are the current stable releases and are well over 14 days old.

Versions and results hold for the toolchain above. The card's tests are the check: run them with your own versions before relying on the fix.

References

1. OWASP OS Command Injection Defense Cheat Sheet
2. Python docs: subprocess - Security Considerations and the shell argument
3. Python docs: subprocess.run
4. Flask docs: request.args (MultiDict)
5. CWE-78: OS Command Injection
6. CWE-88: Argument Injection
7. CAPEC-88: OS Command Injection
8. CAPEC-6: Argument Injection

Attack patterns:

• CAPEC-88 at MITRE
• CAPEC-6 at MITRE