# Command Injection via subprocess.shell

Language: Python
Severity: Critical
CWE: CWE-78

## Source
3, 4

## Flow
3-4-5

## Sink
5

## Vulnerable Code
```python
import subprocess

def trigger_iot_device_reboot(device_mac, reboot_delay):
    device_id = device_mac.replace(':', '')
    cmd = f"curl -X POST https://iot-hub.local/api/reboot?mac={device_id}&delay={reboot_delay}"
    result = subprocess.run(cmd, shell=True, capture_output=True, text=True)
    if result.returncode == 0:
        return {"status": "success", "output": result.stdout}
    return {"status": "failed", "error": result.stderr}
```

## Explanation

The function accepts user-controlled parameters (device_mac and reboot_delay) that are directly interpolated into a shell command string without sanitization. This command is then executed via subprocess.run with shell=True, allowing attackers to inject arbitrary shell commands through metacharacters in either parameter.

## Remediation

The fix eliminates command injection by passing the command as a list of arguments to subprocess.run without shell=True, which prevents shell metacharacter interpretation. Additionally, strict input validation ensures device_mac matches a valid MAC address pattern and reboot_delay is a valid non-negative integer before use.

## Secure Code
```python
import subprocess
import re

def trigger_iot_device_reboot(device_mac, reboot_delay):
    # Validate MAC address format (only hex digits and colons allowed)
    if not re.match(r'^([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}$', device_mac):
        return {"status": "failed", "error": "Invalid MAC address format"}
    
    # Validate reboot_delay is a positive integer
    try:
        delay_value = int(reboot_delay)
        if delay_value < 0:
            return {"status": "failed", "error": "Delay must be a non-negative integer"}
    except (ValueError, TypeError):
        return {"status": "failed", "error": "Invalid delay value"}
    
    device_id = device_mac.replace(':', '')
    
    # Use subprocess with argument list (no shell=True) to prevent command injection
    result = subprocess.run(
        ["curl", "-X", "POST", f"https://iot-hub.local/api/reboot?mac={device_id}&delay={delay_value}"],
        capture_output=True,
        text=True
    )
    if result.returncode == 0:
        return {"status": "success", "output": result.stdout}
    return {"status": "failed", "error": result.stderr}
```