mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-05-26 05:53:33 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -410,8 +410,8 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]:
|
|||||||
try:
|
try:
|
||||||
manifest_raw = zf.read(MANIFEST_NAME).decode("utf-8")
|
manifest_raw = zf.read(MANIFEST_NAME).decode("utf-8")
|
||||||
manifest = json.loads(manifest_raw)
|
manifest = json.loads(manifest_raw)
|
||||||
except (OSError, UnicodeDecodeError, json.JSONDecodeError) as e:
|
except (OSError, UnicodeDecodeError, json.JSONDecodeError):
|
||||||
return False, f"Invalid manifest.json: {e}", {}
|
return False, "Invalid manifest.json", {}
|
||||||
|
|
||||||
if not isinstance(manifest, dict) or "schema_version" not in manifest:
|
if not isinstance(manifest, dict) or "schema_version" not in manifest:
|
||||||
return False, "Invalid manifest structure", {}
|
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
|
return True, "", result_manifest
|
||||||
except zipfile.BadZipFile:
|
except zipfile.BadZipFile:
|
||||||
return False, "File is not a valid ZIP archive", {}
|
return False, "File is not a valid ZIP archive", {}
|
||||||
except OSError as e:
|
except OSError:
|
||||||
return False, f"Could not read backup: {e}", {}
|
return False, "Could not read backup", {}
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ Extracted from PluginManager to improve separation of concerns.
|
|||||||
import json
|
import json
|
||||||
import importlib
|
import importlib
|
||||||
import importlib.util
|
import importlib.util
|
||||||
|
import os
|
||||||
import sys
|
import sys
|
||||||
import subprocess
|
import subprocess
|
||||||
import threading
|
import threading
|
||||||
@@ -68,6 +69,11 @@ class PluginLoader:
|
|||||||
Returns:
|
Returns:
|
||||||
Path to plugin directory or None if not found
|
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
|
# Strategy 1: Use mapping from discovery
|
||||||
if plugin_directories and plugin_id in plugin_directories:
|
if plugin_directories and plugin_id in plugin_directories:
|
||||||
plugin_dir = plugin_directories[plugin_id]
|
plugin_dir = plugin_directories[plugin_id]
|
||||||
@@ -145,6 +151,9 @@ class PluginLoader:
|
|||||||
Returns:
|
Returns:
|
||||||
True if dependencies installed or not needed, False on error
|
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
|
# Resolve and validate plugin_dir before constructing any derived paths
|
||||||
try:
|
try:
|
||||||
plugin_dir_resolved = plugin_dir.resolve(strict=True)
|
plugin_dir_resolved = plugin_dir.resolve(strict=True)
|
||||||
@@ -371,6 +380,9 @@ class PluginLoader:
|
|||||||
Returns:
|
Returns:
|
||||||
Loaded module or None on error
|
Loaded module or None on error
|
||||||
"""
|
"""
|
||||||
|
plugin_id = os.path.basename(plugin_id or '')
|
||||||
|
if not plugin_id:
|
||||||
|
raise PluginError("Invalid plugin ID")
|
||||||
try:
|
try:
|
||||||
plugin_dir_resolved = plugin_dir.resolve(strict=True)
|
plugin_dir_resolved = plugin_dir.resolve(strict=True)
|
||||||
except OSError:
|
except OSError:
|
||||||
|
|||||||
@@ -6939,6 +6939,8 @@ _BACKUP_EXPORT_DIR = PROJECT_ROOT / "config" / "backups" / "exports"
|
|||||||
def _safe_backup_path(filename: str) -> Path:
|
def _safe_backup_path(filename: str) -> Path:
|
||||||
"""Resolve a filename to an absolute path inside the export dir,
|
"""Resolve a filename to an absolute path inside the export dir,
|
||||||
rejecting any traversal attempts. Returns None if unsafe."""
|
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):
|
if not filename or not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9._-]{0,200}\.zip$', filename):
|
||||||
return None
|
return None
|
||||||
path = (_BACKUP_EXPORT_DIR / filename).resolve()
|
path = (_BACKUP_EXPORT_DIR / filename).resolve()
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ from flask import Blueprint, render_template, flash
|
|||||||
from markupsafe import escape
|
from markupsafe import escape
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
|
import re
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from src.web_interface.secret_helpers import mask_secret_fields
|
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.
|
Load plugin configuration partial - server-side rendered form.
|
||||||
This replaces the client-side generateConfigForm() JavaScript.
|
This replaces the client-side generateConfigForm() JavaScript.
|
||||||
"""
|
"""
|
||||||
import re as _re
|
# Sanitize with basename (CodeQL-recognized sanitizer) then regex-validate format
|
||||||
# Reject plugin IDs containing path-traversal characters before any filesystem use
|
plugin_id = os.path.basename(plugin_id or '')
|
||||||
if not _re.match(r'^[a-zA-Z0-9_\-.:]+$', plugin_id or ''):
|
if not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9._\-:]*$', plugin_id):
|
||||||
return '<div class="text-red-500 p-4">Invalid plugin ID</div>', 400
|
return '<div class="text-red-500 p-4">Invalid plugin ID</div>', 400
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -486,8 +488,9 @@ def _load_plugin_config_partial(plugin_id):
|
|||||||
|
|
||||||
def _load_starlark_config_partial(app_id):
|
def _load_starlark_config_partial(app_id):
|
||||||
"""Load configuration partial for a Starlark app."""
|
"""Load configuration partial for a Starlark app."""
|
||||||
import re as _re2
|
# Sanitize with basename (CodeQL-recognized sanitizer) then regex-validate format
|
||||||
if not _re2.match(r'^[a-zA-Z0-9_\-]+$', app_id or ''):
|
app_id = os.path.basename(app_id or '')
|
||||||
|
if not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9_\-]*$', app_id):
|
||||||
return '<div class="text-red-500 p-4">Invalid app ID</div>', 400
|
return '<div class="text-red-500 p-4">Invalid app ID</div>', 400
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user