From 13eaabfcd56a35fac80e3380e43cb745f92b157f Mon Sep 17 00:00:00 2001 From: Chuck Date: Sun, 24 May 2026 08:59:57 -0400 Subject: [PATCH] Fix 15 remaining CodeQL path-injection and stack-trace-exposure alerts Switch from resolve()+relative_to() to os.path.basename() reassignment, which CodeQL recognizes as a path sanitizer that breaks the taint chain. Also remove exception objects from backup_manager validate_backup return strings to eliminate the stack-trace-exposure taint source. Fixes alerts #227, #233, #234, #235, #237, #238, #239, #240, #241, #242, #243, #244, #245, #246, #247. Co-Authored-By: Claude Sonnet 4.6 --- src/backup_manager.py | 8 ++++---- src/plugin_system/plugin_loader.py | 12 ++++++++++++ web_interface/blueprints/api_v3.py | 2 ++ web_interface/blueprints/pages_v3.py | 13 ++++++++----- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/backup_manager.py b/src/backup_manager.py index 86f1cecf..c54ce81e 100644 --- a/src/backup_manager.py +++ b/src/backup_manager.py @@ -410,8 +410,8 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]: try: manifest_raw = zf.read(MANIFEST_NAME).decode("utf-8") manifest = json.loads(manifest_raw) - except (OSError, UnicodeDecodeError, json.JSONDecodeError) as e: - return False, f"Invalid manifest.json: {e}", {} + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + return False, "Invalid manifest.json", {} if not isinstance(manifest, dict) or "schema_version" not in manifest: return False, "Invalid manifest structure", {} @@ -456,8 +456,8 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]: return True, "", result_manifest except zipfile.BadZipFile: return False, "File is not a valid ZIP archive", {} - except OSError as e: - return False, f"Could not read backup: {e}", {} + except OSError: + return False, "Could not read backup", {} # --------------------------------------------------------------------------- diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index 6d3d33f8..75628bb0 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -8,6 +8,7 @@ Extracted from PluginManager to improve separation of concerns. import json import importlib import importlib.util +import os import sys import subprocess import threading @@ -68,6 +69,11 @@ class PluginLoader: Returns: Path to plugin directory or None if not found """ + # Sanitize plugin_id — os.path.basename is a CodeQL-recognized path sanitizer + plugin_id = os.path.basename(plugin_id or '') + if not plugin_id: + return None + # Strategy 1: Use mapping from discovery if plugin_directories and plugin_id in plugin_directories: plugin_dir = plugin_directories[plugin_id] @@ -145,6 +151,9 @@ class PluginLoader: Returns: True if dependencies installed or not needed, False on error """ + plugin_id = os.path.basename(plugin_id or '') + if not plugin_id: + return False # Resolve and validate plugin_dir before constructing any derived paths try: plugin_dir_resolved = plugin_dir.resolve(strict=True) @@ -371,6 +380,9 @@ class PluginLoader: Returns: Loaded module or None on error """ + plugin_id = os.path.basename(plugin_id or '') + if not plugin_id: + raise PluginError("Invalid plugin ID") try: plugin_dir_resolved = plugin_dir.resolve(strict=True) except OSError: diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index b27a4170..7949923b 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -6939,6 +6939,8 @@ _BACKUP_EXPORT_DIR = PROJECT_ROOT / "config" / "backups" / "exports" def _safe_backup_path(filename: str) -> Path: """Resolve a filename to an absolute path inside the export dir, rejecting any traversal attempts. Returns None if unsafe.""" + # Use basename first (CodeQL-recognized sanitizer) then validate format + filename = os.path.basename(filename or '') if not filename or not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9._-]{0,200}\.zip$', filename): return None path = (_BACKUP_EXPORT_DIR / filename).resolve() diff --git a/web_interface/blueprints/pages_v3.py b/web_interface/blueprints/pages_v3.py index a1e119aa..67dc7d6d 100644 --- a/web_interface/blueprints/pages_v3.py +++ b/web_interface/blueprints/pages_v3.py @@ -2,6 +2,8 @@ from flask import Blueprint, render_template, flash from markupsafe import escape import json import logging +import os +import re from pathlib import Path from src.web_interface.secret_helpers import mask_secret_fields @@ -353,9 +355,9 @@ def _load_plugin_config_partial(plugin_id): Load plugin configuration partial - server-side rendered form. This replaces the client-side generateConfigForm() JavaScript. """ - import re as _re - # Reject plugin IDs containing path-traversal characters before any filesystem use - if not _re.match(r'^[a-zA-Z0-9_\-.:]+$', plugin_id or ''): + # Sanitize with basename (CodeQL-recognized sanitizer) then regex-validate format + plugin_id = os.path.basename(plugin_id or '') + if not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9._\-:]*$', plugin_id): return '
Invalid plugin ID
', 400 try: @@ -486,8 +488,9 @@ def _load_plugin_config_partial(plugin_id): def _load_starlark_config_partial(app_id): """Load configuration partial for a Starlark app.""" - import re as _re2 - if not _re2.match(r'^[a-zA-Z0-9_\-]+$', app_id or ''): + # Sanitize with basename (CodeQL-recognized sanitizer) then regex-validate format + app_id = os.path.basename(app_id or '') + if not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9_\-]*$', app_id): return '
Invalid app ID
', 400 try: