Security: Add path traversal protection to plugin operations
Add defense-in-depth validation to prevent path traversal attacks when enabling/installing plugins via the UI or CLI. Layer 1 - Input Validation (enable_plugin): - Validate plugin names with regex ^[a-zA-Z0-9_-]+$ - Reject reserved names (., .., empty string) - Blocks attempts like ../../../etc/passwd at entry point Layer 2 - Path Boundary Check (load_manifest): - Verify resolved manifest path stays within plugins_dir - Defense in depth even if name validation is bypassed - Compatible with Python 3.8+ (fallback for < 3.9) Risk Assessment: LOW - Requires admin role (already trusted) - Multiple existing validation layers - No known exploitation in the wild Rationale: Defense in depth best practice. Admins are trusted, but input validation should not rely solely on authorization checks. Tested: - Valid names pass (backups, my-plugin, plugin-name) - Path traversal blocked (../../../etc/passwd) - Reserved names blocked (..) - Both validation layers confirmed working Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ from pathlib import Path
|
||||
from typing import Dict, List, Optional
|
||||
from flask import Flask
|
||||
import logging
|
||||
import re
|
||||
|
||||
from .base import BasePlugin, PluginMeta
|
||||
from .registry import PluginRegistry, PluginState
|
||||
@@ -469,6 +470,17 @@ class PluginManager:
|
||||
|
||||
def enable_plugin(self, name: str) -> bool:
|
||||
"""Enable a disabled plugin. Auto-installs if available but not installed."""
|
||||
# Security: Validate plugin name to prevent path traversal
|
||||
# Only allow alphanumeric, underscore, and hyphen (matches existing plugins)
|
||||
if not re.match(r'^[a-zA-Z0-9_-]+$', name):
|
||||
logger.error(f"Invalid plugin name format: {name}")
|
||||
return False
|
||||
|
||||
# Reject reserved names that could be used for directory traversal
|
||||
if name in ('.', '..', ''):
|
||||
logger.error(f"Invalid plugin name: {name}")
|
||||
return False
|
||||
|
||||
# Auto-install if available but not installed (UX: enabling should just work)
|
||||
if not self.registry.is_installed(name):
|
||||
logger.info(f"Plugin {name} not installed, installing first")
|
||||
|
||||
@@ -84,6 +84,29 @@ class PluginLoader:
|
||||
return self._manifests[name]
|
||||
|
||||
manifest_path = self.plugins_dir / name / 'manifest.json'
|
||||
|
||||
# Security: Ensure resolved path stays within plugins_dir boundary
|
||||
# Defense in depth against path traversal, even if name was validated
|
||||
resolved_manifest = manifest_path.resolve()
|
||||
resolved_plugins = self.plugins_dir.resolve()
|
||||
|
||||
# Python 3.9+ has is_relative_to, fallback for older versions
|
||||
try:
|
||||
if not resolved_manifest.is_relative_to(resolved_plugins):
|
||||
raise PluginNotFoundError(
|
||||
f'Plugin path escapes plugins directory: {name}',
|
||||
plugin_name=name,
|
||||
)
|
||||
except AttributeError:
|
||||
# Python < 3.9 fallback using relative_to
|
||||
try:
|
||||
resolved_manifest.relative_to(resolved_plugins)
|
||||
except ValueError:
|
||||
raise PluginNotFoundError(
|
||||
f'Plugin path escapes plugins directory: {name}',
|
||||
plugin_name=name,
|
||||
)
|
||||
|
||||
if not manifest_path.exists():
|
||||
raise PluginNotFoundError(
|
||||
f'Plugin {name} has no manifest.json at {manifest_path}',
|
||||
|
||||
Reference in New Issue
Block a user