🔴 CRITICAL: Fix OS Command Injection in Invoice Generation (Line 278)

Security Vulnerability: OS Command Injection (CWE-78)

Severity: HIGH
Priority: CRITICAL - Fix Today
Location: app.py, line 278
Vulnerability ID: 238776719


📋 Summary

The /demo/generate_invoice.html endpoint contains a critical OS command injection vulnerability where user-controlled input (customer_name) is directly passed to subprocess.run() with shell=True, allowing arbitrary command execution.

🎯 Exploitability Analysis

Risk Level: HIGHLY EXPLOITABLE

Code Flow:

  • Source: User input at line 246 (customer_name from form)
  • Propagation: Line 246 → Line 275
  • Sink: Command execution at lines 278-282

Attack Vector:

# Attacker submits:
customer_name = "John'; cat /etc/passwd #"

# Results in command:
echo '...' > path && echo 'Invoice generated for John'; cat /etc/passwd #'

💥 Impact

  • Complete system compromise
  • Arbitrary code execution on the server
  • Data exfiltration (database, credentials, source code)
  • Denial of service attacks
  • Lateral movement within infrastructure

🔧 Required Fix

Replace the vulnerable code with safe subprocess usage:

Current Vulnerable Code (Line 278):

vulnerable_command = f"echo '{invoice_content}' > {invoice_path} && echo 'Invoice generated for {customer_name}'"
result = subprocess.run(vulnerable_command, shell=True, capture_output=True, text=True, timeout=10)

Secure Implementation:

# Option 1: Use argument list (NO shell=True)
with open(invoice_path, 'w') as f:
    f.write(invoice_content)
result = subprocess.run(['echo', f'Invoice generated for {customer_name}'], 
                       capture_output=True, 
                       text=True)

# Option 2: Better - avoid subprocess entirely
with open(invoice_path, 'w') as f:
    f.write(invoice_content)
command_output = f'Invoice generated for {customer_name}'

Acceptance Criteria

  • Remove shell=True from subprocess.run()
  • Use argument list format instead of string concatenation
  • Validate and sanitize customer_name input
  • Add input length limits
  • Test with malicious payloads to verify fix
  • Update security tests

📚 References


Timeline: Fix required TODAY - this is actively exploitable