# Command Injection via subprocess.shell=True

Language: Python
Severity: Critical
CWE: CWE-78

## Source
3

## Flow
3-4

## Sink
4

## Vulnerable Code
```python
import subprocess

def provision_iot_device(device_mac, firmware_url):
    cmd = f"curl -o /tmp/{device_mac}.bin {firmware_url} && md5sum /tmp/{device_mac}.bin"
    result = subprocess.run(cmd, shell=True, capture_output=True, text=True)
    if result.returncode == 0:
        return {"status": "success", "checksum": result.stdout.strip()}
    return {"status": "failed", "error": result.stderr}
```

## Explanation

The function constructs a shell command using f-string interpolation with unsanitized user inputs (device_mac and firmware_url), then executes it via subprocess.run() with shell=True. An attacker can inject arbitrary shell commands through specially crafted parameters containing shell metacharacters like semicolons, pipes, or backticks.

## Remediation

The fix eliminates command injection by: (1) validating device_mac against a strict regex pattern for MAC addresses, (2) validating the firmware URL scheme, (3) using subprocess.run with a list of arguments instead of shell=True which prevents shell metacharacter interpretation, and (4) replacing the md5sum shell command with Python's hashlib for checksum calculation.

## Secure Code
```python
import subprocess
import re
import os
import hashlib
import urllib.request
import urllib.parse


def provision_iot_device(device_mac, firmware_url):
    # Validate MAC address format (only allow hex digits and colons/hyphens)
    if not re.match(r'^[0-9A-Fa-f]{2}([:\-][0-9A-Fa-f]{2}){5}$', device_mac):
        return {"status": "failed", "error": "Invalid MAC address format"}

    # Validate firmware URL scheme
    parsed_url = urllib.parse.urlparse(firmware_url)
    if parsed_url.scheme not in ('http', 'https'):
        return {"status": "failed", "error": "Invalid firmware URL scheme"}

    # Sanitize MAC for use as filename (replace colons/hyphens with underscores)
    safe_mac = device_mac.replace(':', '_').replace('-', '_')
    output_path = os.path.join('/tmp', f"{safe_mac}.bin")

    # Download firmware using subprocess without shell=True
    try:
        download_result = subprocess.run(
            ['curl', '-o', output_path, '--', firmware_url],
            capture_output=True, text=True, timeout=120
        )
        if download_result.returncode != 0:
            return {"status": "failed", "error": download_result.stderr}

        # Calculate MD5 checksum using Python's hashlib instead of shell command
        md5_hash = hashlib.md5()
        with open(output_path, 'rb') as f:
            for chunk in iter(lambda: f.read(8192), b''):
                md5_hash.update(chunk)
        checksum = md5_hash.hexdigest()

        return {"status": "success", "checksum": f"{checksum}  {output_path}"}
    except subprocess.TimeoutExpired:
        return {"status": "failed", "error": "Download timed out"}
    except FileNotFoundError:
        return {"status": "failed", "error": "curl not found or file not downloaded"}
    except Exception as e:
        return {"status": "failed", "error": str(e)}
```