fix: path traversal vulnerability - use prefix matching in is_dangerous_path()
The is_dangerous_path() function only did exact string matching, allowing attackers to bypass protection by accessing subdirectories: - /etc was blocked but /etc/passwd was allowed - C:\Windows was blocked but C:\Windows\System32\... was allowed This minimal fix changes is_dangerous_path() to use PREFIX MATCHING: - Now blocks dangerous directories AND all their subdirectories - Paths like /etcbackup are still allowed (not under /etc) - No changes to DANGEROUS_PATHS list Security: - Fixes CWE-22: Path Traversal vulnerability - Reported by: Team off-course (K-Shield.Jr 15th) Fixes #312 Fixes #293
This commit is contained in:
65
tests/test_path_traversal_security.py
Normal file
65
tests/test_path_traversal_security.py
Normal file
@@ -0,0 +1,65 @@
|
|||||||
|
"""
|
||||||
|
Test path traversal security fix.
|
||||||
|
|
||||||
|
Fixes vulnerability reported in:
|
||||||
|
- https://github.com/BeehiveInnovations/zen-mcp-server/issues/293
|
||||||
|
- https://github.com/BeehiveInnovations/zen-mcp-server/issues/312
|
||||||
|
|
||||||
|
The vulnerability: is_dangerous_path() only did exact string matching,
|
||||||
|
so /etc was blocked but /etc/passwd was allowed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from utils.security_config import is_dangerous_path
|
||||||
|
|
||||||
|
|
||||||
|
class TestPathTraversalFix:
|
||||||
|
"""Test that subdirectories of dangerous paths are now blocked."""
|
||||||
|
|
||||||
|
def test_exact_match_still_works(self):
|
||||||
|
"""Test that exact dangerous paths are still blocked."""
|
||||||
|
assert is_dangerous_path(Path("/etc")) is True
|
||||||
|
assert is_dangerous_path(Path("/usr")) is True
|
||||||
|
assert is_dangerous_path(Path("/var")) is True
|
||||||
|
|
||||||
|
def test_subdirectory_now_blocked(self):
|
||||||
|
"""Test that subdirectories are now blocked (the fix)."""
|
||||||
|
# These were allowed before the fix
|
||||||
|
assert is_dangerous_path(Path("/etc/passwd")) is True
|
||||||
|
assert is_dangerous_path(Path("/etc/shadow")) is True
|
||||||
|
assert is_dangerous_path(Path("/etc/hosts")) is True
|
||||||
|
assert is_dangerous_path(Path("/var/log/auth.log")) is True
|
||||||
|
|
||||||
|
def test_deeply_nested_blocked(self):
|
||||||
|
"""Test that deeply nested paths are blocked."""
|
||||||
|
assert is_dangerous_path(Path("/etc/ssh/sshd_config")) is True
|
||||||
|
assert is_dangerous_path(Path("/usr/local/bin/python")) is True
|
||||||
|
|
||||||
|
def test_root_blocked(self):
|
||||||
|
"""Test that root directory is blocked."""
|
||||||
|
assert is_dangerous_path(Path("/")) is True
|
||||||
|
|
||||||
|
def test_safe_paths_allowed(self):
|
||||||
|
"""Test that safe paths are still allowed."""
|
||||||
|
# User project directories should be allowed
|
||||||
|
assert is_dangerous_path(Path("/tmp/test")) is False
|
||||||
|
assert is_dangerous_path(Path("/tmp/myproject/src")) is False
|
||||||
|
|
||||||
|
def test_similar_names_not_blocked(self):
|
||||||
|
"""Test that paths with similar names are not blocked."""
|
||||||
|
# /etcbackup should NOT be blocked (it's not under /etc)
|
||||||
|
assert is_dangerous_path(Path("/tmp/etcbackup")) is False
|
||||||
|
assert is_dangerous_path(Path("/tmp/my_etc_files")) is False
|
||||||
|
|
||||||
|
|
||||||
|
class TestRegressionPrevention:
|
||||||
|
"""Regression tests for the specific vulnerability."""
|
||||||
|
|
||||||
|
def test_etc_passwd_blocked(self):
|
||||||
|
"""Test /etc/passwd is blocked (common attack target)."""
|
||||||
|
assert is_dangerous_path(Path("/etc/passwd")) is True
|
||||||
|
|
||||||
|
def test_etc_shadow_blocked(self):
|
||||||
|
"""Test /etc/shadow is blocked (password hashes)."""
|
||||||
|
assert is_dangerous_path(Path("/etc/shadow")) is True
|
||||||
@@ -89,16 +89,47 @@ EXCLUDED_DIRS = {
|
|||||||
|
|
||||||
def is_dangerous_path(path: Path) -> bool:
|
def is_dangerous_path(path: Path) -> bool:
|
||||||
"""
|
"""
|
||||||
Check if a path is in the dangerous paths list.
|
Check if a path is in or under a dangerous directory.
|
||||||
|
|
||||||
|
Uses PREFIX MATCHING to block dangerous directories AND their subdirectories.
|
||||||
|
For example, if "/etc" is in DANGEROUS_PATHS, both "/etc" and "/etc/passwd"
|
||||||
|
will be blocked.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
path: Path to check
|
path: Path to check
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
True if the path is dangerous and should not be accessed
|
True if the path is dangerous and should not be accessed
|
||||||
|
|
||||||
|
Security:
|
||||||
|
Fixes path traversal vulnerability (CWE-22) reported in:
|
||||||
|
- https://github.com/BeehiveInnovations/zen-mcp-server/issues/293
|
||||||
|
- https://github.com/BeehiveInnovations/zen-mcp-server/issues/312
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
resolved = path.resolve()
|
resolved = path.resolve()
|
||||||
return str(resolved) in DANGEROUS_PATHS or resolved.parent == resolved
|
resolved_str = str(resolved)
|
||||||
|
|
||||||
|
# Check 1: Root directory (filesystem root)
|
||||||
|
if resolved.parent == resolved:
|
||||||
|
return True
|
||||||
|
|
||||||
|
# Check 2: Exact match or subdirectory of dangerous paths
|
||||||
|
for dangerous in DANGEROUS_PATHS:
|
||||||
|
# Skip root "/" - already handled above
|
||||||
|
if dangerous == "/":
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Exact match
|
||||||
|
if resolved_str == dangerous:
|
||||||
|
return True
|
||||||
|
|
||||||
|
# Subdirectory check: path starts with dangerous + separator
|
||||||
|
# Use os.sep for platform-appropriate separator
|
||||||
|
if resolved_str.startswith(dangerous + "/") or resolved_str.startswith(dangerous + "\\"):
|
||||||
|
return True
|
||||||
|
|
||||||
|
return False
|
||||||
|
|
||||||
except Exception:
|
except Exception:
|
||||||
return True # If we can't resolve, consider it dangerous
|
return True # If we can't resolve, consider it dangerous
|
||||||
|
|||||||
Reference in New Issue
Block a user