From d31fb7846bcbfc9e2e978d21045586b8ebb5c99f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 03:24:55 +0000 Subject: [PATCH 1/4] Initial plan From 4ebf1214f35493d75aa285d35bc61db47ccd0af8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 03:35:30 +0000 Subject: [PATCH 2/4] Fix security vulnerabilities: command injection, hardcoded secrets, path traversal, insecure temp files Co-authored-by: P4X-ng <223870169+P4X-ng@users.noreply.github.com> --- scripts/recovery/autonuke.py | 37 ++++++++++++++----- scripts/recovery/hardware-recovery.sh | 2 +- scripts/recovery/nuclear-wipe.sh | 21 +++++++++++ .../secure-boot/enable-secureboot-kexec.sh | 2 +- scripts/secure-boot/keys-centralize.sh | 2 +- scripts/uefi-tools/uuefi-apply.sh | 2 +- scripts/validation/detect_bootkit.py | 2 +- web/hardware_database_server.py | 8 +++- 8 files changed, 61 insertions(+), 15 deletions(-) diff --git a/scripts/recovery/autonuke.py b/scripts/recovery/autonuke.py index e53c30a..e192d0a 100755 --- a/scripts/recovery/autonuke.py +++ b/scripts/recovery/autonuke.py @@ -113,17 +113,36 @@ def confirm_action(self, message: str, danger_level: str = "LOW") -> bool: response = input(f"{Colors.CYAN}Continue? [y/N]: {Colors.END}").strip().lower() return response in ['y', 'yes'] - def run_command(self, cmd: str, shell: bool = True) -> Tuple[int, str, str]: - """Run a command and return exit code, stdout, stderr""" + def run_command(self, cmd: str, shell: bool = False) -> Tuple[int, str, str]: + """Run a command and return exit code, stdout, stderr + + Args: + cmd: Command string (will be split into list if shell=False) + shell: If True, runs command in shell (use only for trusted input) + + Note: For security, shell=False is default. Most uses split the command string. + """ self.log(f"Executing: {cmd}") try: - result = subprocess.run( - cmd, - shell=shell, - capture_output=True, - text=True, - cwd=self.project_root - ) + if shell: + # Only use shell mode for trusted, hardcoded commands + result = subprocess.run( + cmd, + shell=True, + capture_output=True, + text=True, + cwd=self.project_root + ) + else: + # Safer mode: split command into list + cmd_list = cmd.split() + result = subprocess.run( + cmd_list, + shell=False, + capture_output=True, + text=True, + cwd=self.project_root + ) return result.returncode, result.stdout, result.stderr except Exception as e: self.log(f"Command failed: {e}", "ERROR") diff --git a/scripts/recovery/hardware-recovery.sh b/scripts/recovery/hardware-recovery.sh index dc8dbbe..cb354cb 100755 --- a/scripts/recovery/hardware-recovery.sh +++ b/scripts/recovery/hardware-recovery.sh @@ -82,7 +82,7 @@ if [ -f "$FIRMWARE_IMAGE" ]; then [ -n "$VERBOSE" ] && ARGS="$ARGS -v" [ -n "$VERIFY_ONLY" ] && ARGS="$ARGS --verify-only" - sudo python3 scripts/hardware_firmware_recovery.py $ARGS --output hardware_recovery_results.json + sudo python3 scripts/hardware_firmware_recovery.py "$ARGS" --output hardware_recovery_results.json else echo "ERROR: Clean firmware image not found at $FIRMWARE_IMAGE" echo " This must be your EXACT hardware's clean firmware dump." diff --git a/scripts/recovery/nuclear-wipe.sh b/scripts/recovery/nuclear-wipe.sh index 9f5f796..8c6167c 100755 --- a/scripts/recovery/nuclear-wipe.sh +++ b/scripts/recovery/nuclear-wipe.sh @@ -94,6 +94,13 @@ case "$choice" in exit 1 fi + # Validate device path format for security + if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then + echo "❌ Invalid device path format: $device" + echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + exit 1 + fi + echo "⚠️ FINAL CONFIRMATION" echo " Device: $device" echo " Method: Quick wipe (zeros)" @@ -115,6 +122,13 @@ case "$choice" in exit 1 fi + # Validate device path format for security + if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then + echo "❌ Invalid device path format: $device" + echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + exit 1 + fi + echo "⚠️ FINAL CONFIRMATION" echo " Device: $device" echo " Method: DoD Short (3 passes)" @@ -137,6 +151,13 @@ case "$choice" in exit 1 fi + # Validate device path format for security + if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then + echo "❌ Invalid device path format: $device" + echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + exit 1 + fi + echo "⚠️ FINAL CONFIRMATION" echo " Device: $device" echo " Method: PRNG Stream (cryptographically secure)" diff --git a/scripts/secure-boot/enable-secureboot-kexec.sh b/scripts/secure-boot/enable-secureboot-kexec.sh index 6cb77e6..732ee95 100755 --- a/scripts/secure-boot/enable-secureboot-kexec.sh +++ b/scripts/secure-boot/enable-secureboot-kexec.sh @@ -220,7 +220,7 @@ if [[ ! $REPLY =~ ^[Yy]$ ]]; then fi # Create a script that will run after first kexec -TEMP_SCRIPT="/tmp/phoenixboot_secureboot_enable_phase2.sh" +TEMP_SCRIPT=$(mktemp /tmp/phoenixboot_secureboot_enable_phase2.XXXXXX.sh) cat > "$TEMP_SCRIPT" << 'EOF' #!/bin/bash diff --git a/scripts/secure-boot/keys-centralize.sh b/scripts/secure-boot/keys-centralize.sh index 017c25f..52f7f01 100755 --- a/scripts/secure-boot/keys-centralize.sh +++ b/scripts/secure-boot/keys-centralize.sh @@ -23,7 +23,7 @@ DRY_RUN=${DRY_RUN:-0} PRUNE=${PRUNE:-0} if [ "${1:-}" = "--prune" ]; then PRUNE=1; fi -run() { if [ "$DRY_RUN" = 1 ]; then echo "DRY: $*"; else eval "$*"; fi } +run() { if [ "$DRY_RUN" = 1 ]; then echo "DRY: $*"; else "$@"; fi } move_if_exists() { local src="$1" dest_dir="$2" diff --git a/scripts/uefi-tools/uuefi-apply.sh b/scripts/uefi-tools/uuefi-apply.sh index ddef444..e2febcc 100755 --- a/scripts/uefi-tools/uuefi-apply.sh +++ b/scripts/uefi-tools/uuefi-apply.sh @@ -7,7 +7,7 @@ info "☠ UUEFI apply (set BootNext for selected app)" # Dry-run mode: UUEFI_DRYRUN=1 DRY=${UUEFI_DRYRUN:-} -run() { if [ -n "$DRY" ]; then echo "DRYRUN: $*"; else eval "$*"; fi } +run() { if [ -n "$DRY" ]; then echo "DRYRUN: $*"; else "$@"; fi } need_sudo() { if [ -n "$DRY" ]; then echo sudo -n "$@" || true; else sudo -n "$@"; fi } diff --git a/scripts/validation/detect_bootkit.py b/scripts/validation/detect_bootkit.py index 07071b0..1e1b46b 100755 --- a/scripts/validation/detect_bootkit.py +++ b/scripts/validation/detect_bootkit.py @@ -372,7 +372,7 @@ def main(): time.sleep(10) # Trigger recovery - os.system("sudo make reboot-to-vm") + subprocess.run(["sudo", "make", "reboot-to-vm"], check=False) return 0 diff --git a/web/hardware_database_server.py b/web/hardware_database_server.py index 03e9975..53d0331 100644 --- a/web/hardware_database_server.py +++ b/web/hardware_database_server.py @@ -17,14 +17,16 @@ from flask import Flask, request, jsonify, render_template_string, send_file import json import os +import secrets from datetime import datetime from pathlib import Path from typing import Dict, List import sqlite3 import hashlib +import re app = Flask(__name__) -app.config['SECRET_KEY'] = 'phoenix_guard_hardware_db' +app.config['SECRET_KEY'] = os.environ.get('SECRET_KEY', secrets.token_hex(32)) # Database setup DB_PATH = Path("hardware_profiles.db") @@ -512,6 +514,10 @@ def search_hardware(): @app.route('/api/download/') def download_config(hardware_id): """Download universal BIOS config for hardware""" + # Validate hardware_id to prevent path traversal + if not re.match(r'^[a-zA-Z0-9_-]+$', hardware_id): + return "Invalid hardware ID format", 400 + profile_file = UPLOADS_PATH / f"{hardware_id}.json" if not profile_file.exists(): From 6376e69f11e256dc7074850b3b3a2a5cc454c62b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 03:37:08 +0000 Subject: [PATCH 3/4] Fix shellcheck error: use loop instead of glob with -f test Co-authored-by: P4X-ng <223870169+P4X-ng@users.noreply.github.com> --- scripts/secure-boot/enable-secureboot-kexec.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/secure-boot/enable-secureboot-kexec.sh b/scripts/secure-boot/enable-secureboot-kexec.sh index 732ee95..f7013a8 100755 --- a/scripts/secure-boot/enable-secureboot-kexec.sh +++ b/scripts/secure-boot/enable-secureboot-kexec.sh @@ -54,13 +54,14 @@ fi # Check if Secure Boot is already enabled check_secureboot_enabled() { - if [ -f /sys/firmware/efi/efivars/SecureBoot-* ]; then - local sb_file=$(ls /sys/firmware/efi/efivars/SecureBoot-* 2>/dev/null | head -1) - if [ -n "$sb_file" ]; then - local sb_status=$(od -An -t u1 -j 4 -N 1 "$sb_file" 2>/dev/null | tr -d ' ') + local sb_file + for sb_file in /sys/firmware/efi/efivars/SecureBoot-*; do + if [ -f "$sb_file" ]; then + local sb_status + sb_status=$(od -An -t u1 -j 4 -N 1 "$sb_file" 2>/dev/null | tr -d ' ') [ "$sb_status" = "1" ] && return 0 fi - fi + done return 1 } From 6d5a4e17c9ff7bbbadf518a20e5c7b9d2e11745d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 03:40:59 +0000 Subject: [PATCH 4/4] Address code review feedback: fix TOCTOU, shlex.split, DRY violation, and ARGS word splitting Co-authored-by: P4X-ng <223870169+P4X-ng@users.noreply.github.com> --- scripts/recovery/autonuke.py | 8 +++++--- scripts/recovery/hardware-recovery.sh | 3 ++- scripts/recovery/nuclear-wipe.sh | 23 ++++++++++++++--------- web/hardware_database_server.py | 8 ++++++++ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/scripts/recovery/autonuke.py b/scripts/recovery/autonuke.py index e192d0a..42e7182 100755 --- a/scripts/recovery/autonuke.py +++ b/scripts/recovery/autonuke.py @@ -21,6 +21,7 @@ import sys import json import subprocess +import shlex import time from pathlib import Path from typing import Dict, List, Tuple, Optional @@ -120,7 +121,8 @@ def run_command(self, cmd: str, shell: bool = False) -> Tuple[int, str, str]: cmd: Command string (will be split into list if shell=False) shell: If True, runs command in shell (use only for trusted input) - Note: For security, shell=False is default. Most uses split the command string. + Note: For security, shell=False is default. Command is split using shlex.split() + which properly handles quoted arguments and special characters. """ self.log(f"Executing: {cmd}") try: @@ -134,8 +136,8 @@ def run_command(self, cmd: str, shell: bool = False) -> Tuple[int, str, str]: cwd=self.project_root ) else: - # Safer mode: split command into list - cmd_list = cmd.split() + # Safer mode: split command using shlex for proper quoting + cmd_list = shlex.split(cmd) result = subprocess.run( cmd_list, shell=False, diff --git a/scripts/recovery/hardware-recovery.sh b/scripts/recovery/hardware-recovery.sh index cb354cb..f336334 100755 --- a/scripts/recovery/hardware-recovery.sh +++ b/scripts/recovery/hardware-recovery.sh @@ -82,7 +82,8 @@ if [ -f "$FIRMWARE_IMAGE" ]; then [ -n "$VERBOSE" ] && ARGS="$ARGS -v" [ -n "$VERIFY_ONLY" ] && ARGS="$ARGS --verify-only" - sudo python3 scripts/hardware_firmware_recovery.py "$ARGS" --output hardware_recovery_results.json + # shellcheck disable=SC2086 + sudo python3 scripts/hardware_firmware_recovery.py $ARGS --output hardware_recovery_results.json else echo "ERROR: Clean firmware image not found at $FIRMWARE_IMAGE" echo " This must be your EXACT hardware's clean firmware dump." diff --git a/scripts/recovery/nuclear-wipe.sh b/scripts/recovery/nuclear-wipe.sh index 8c6167c..f9d1018 100755 --- a/scripts/recovery/nuclear-wipe.sh +++ b/scripts/recovery/nuclear-wipe.sh @@ -18,6 +18,17 @@ if [ "$EUID" -ne 0 ]; then exit 1 fi +# Validate device path format for security +validate_device_path() { + local device="$1" + if [[ ! "$device" =~ ^/dev/(sd[a-z]{1,4}|nvme[0-9]+n[0-9]+|vd[a-z]{1,2}|mmcblk[0-9]+)$ ]]; then + echo "❌ Invalid device path format: $device" + echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + return 1 + fi + return 0 +} + # Check if nwipe is installed if ! command -v nwipe &> /dev/null; then echo "⚠️ nwipe not found - attempting to install..." @@ -95,9 +106,7 @@ case "$choice" in fi # Validate device path format for security - if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then - echo "❌ Invalid device path format: $device" - echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + if ! validate_device_path "$device"; then exit 1 fi @@ -123,9 +132,7 @@ case "$choice" in fi # Validate device path format for security - if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then - echo "❌ Invalid device path format: $device" - echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + if ! validate_device_path "$device"; then exit 1 fi @@ -152,9 +159,7 @@ case "$choice" in fi # Validate device path format for security - if [[ ! "$device" =~ ^/dev/(sd[a-z]+|nvme[0-9]+n[0-9]+|vd[a-z]+|mmcblk[0-9]+)$ ]]; then - echo "❌ Invalid device path format: $device" - echo " Expected format: /dev/sdX, /dev/nvmeXnY, /dev/vdX, or /dev/mmcblkX" + if ! validate_device_path "$device"; then exit 1 fi diff --git a/web/hardware_database_server.py b/web/hardware_database_server.py index 53d0331..59c4c35 100644 --- a/web/hardware_database_server.py +++ b/web/hardware_database_server.py @@ -520,6 +520,14 @@ def download_config(hardware_id): profile_file = UPLOADS_PATH / f"{hardware_id}.json" + # Resolve symlinks and verify path stays within UPLOADS_PATH + try: + profile_file = profile_file.resolve() + if not str(profile_file).startswith(str(UPLOADS_PATH.resolve())): + return "Invalid file path", 400 + except (OSError, RuntimeError): + return "Invalid file path", 400 + if not profile_file.exists(): return "Profile not found", 404