# Command Injection via subprocess.shell=True

Language: Python
Severity: Critical
CWE: CWE-78

## Source
10-11

## Flow
10-11-12-14

## Sink
14

## Vulnerable Code
```python
import subprocess
import logging
from flask import Flask, request, jsonify

app = Flask(__name__)
logger = logging.getLogger(__name__)

@app.route('/api/iot/device/reboot', methods=['POST'])
def trigger_device_reboot():
    device_mac = request.json.get('mac_address', '')
    reboot_delay = request.json.get('delay_seconds', '0')
    logger.info(f"Initiating reboot for device: {device_mac}")
    wake_cmd = f"wakeonlan {device_mac} && sleep {reboot_delay} && ssh root@{device_mac} 'reboot now'"
    try:
        result = subprocess.run(wake_cmd, shell=True, capture_output=True, timeout=30, text=True)
        return jsonify({
            'status': 'success',
            'device': device_mac,
            'output': result.stdout
        }), 200
    except subprocess.TimeoutExpired:
        return jsonify({'status': 'error', 'message': 'Reboot command timed out'}), 500
    except Exception as e:
        return jsonify({'status': 'error', 'message': str(e)}), 500
```

## Explanation

The application takes unsanitized user input from request.json (device_mac and reboot_delay) and directly concatenates them into a shell command string that is executed via subprocess.run() with shell=True. This allows attackers to inject arbitrary shell commands through either parameter, leading to remote code execution on the server.

## Remediation

The fix eliminates command injection by replacing the single shell=True call with separate subprocess.run() calls using argument lists (shell=False), which prevents shell interpretation of special characters. Additionally, strict input validation is applied: MAC addresses are validated against a regex pattern, and the delay parameter is validated as a bounded integer, ensuring no malicious payloads can be passed even to the argument-list based calls.

## Secure Code
```python
import subprocess
import logging
import re
from flask import Flask, request, jsonify

app = Flask(__name__)
logger = logging.getLogger(__name__)

MAC_ADDRESS_REGEX = re.compile(r'^([0-9A-Fa-f]{2}[:\-]){5}([0-9A-Fa-f]{2})$')

def validate_mac_address(mac):
    """Validate that the input is a properly formatted MAC address."""
    if not mac or not MAC_ADDRESS_REGEX.match(mac):
        return False
    return True

def validate_delay(delay):
    """Validate that delay is a non-negative integer within acceptable range."""
    try:
        delay_int = int(delay)
        if delay_int < 0 or delay_int > 300:
            return False
        return True
    except (ValueError, TypeError):
        return False

@app.route('/api/iot/device/reboot', methods=['POST'])
def trigger_device_reboot():
    device_mac = request.json.get('mac_address', '')
    reboot_delay = request.json.get('delay_seconds', '0')

    if not validate_mac_address(device_mac):
        return jsonify({'status': 'error', 'message': 'Invalid MAC address format'}), 400

    if not validate_delay(reboot_delay):
        return jsonify({'status': 'error', 'message': 'Invalid delay value. Must be an integer between 0 and 300.'}), 400

    delay_seconds = str(int(reboot_delay))

    logger.info(f"Initiating reboot for device: {device_mac}")

    try:
        # Step 1: Wake-on-LAN
        wol_result = subprocess.run(
            ['wakeonlan', device_mac],
            capture_output=True, timeout=10, text=True
        )
        if wol_result.returncode != 0:
            return jsonify({'status': 'error', 'message': 'Wake-on-LAN failed', 'output': wol_result.stderr}), 500

        # Step 2: Sleep for the specified delay
        if int(delay_seconds) > 0:
            subprocess.run(
                ['sleep', delay_seconds],
                capture_output=True, timeout=int(delay_seconds) + 5
            )

        # Step 3: SSH reboot command
        ssh_result = subprocess.run(
            ['ssh', f'root@{device_mac}', 'reboot', 'now'],
            capture_output=True, timeout=15, text=True
        )

        return jsonify({
            'status': 'success',
            'device': device_mac,
            'output': wol_result.stdout + ssh_result.stdout
        }), 200
    except subprocess.TimeoutExpired:
        return jsonify({'status': 'error', 'message': 'Reboot command timed out'}), 500
    except Exception as e:
        logger.error(f"Reboot failed for {device_mac}: {str(e)}")
        return jsonify({'status': 'error', 'message': 'An internal error occurred'}), 500
```