From 9ed15f405a9462b4db7aa44ca2d989e092c008e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E6=A0=8B=E6=A2=81?= Date: Wed, 3 Dec 2025 15:17:07 +0800 Subject: [PATCH] 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 --- tests/test_path_traversal_security.py | 65 +++++++++++++++++++++++++++ utils/security_config.py | 35 ++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 tests/test_path_traversal_security.py diff --git a/tests/test_path_traversal_security.py b/tests/test_path_traversal_security.py new file mode 100644 index 0000000..1345398 --- /dev/null +++ b/tests/test_path_traversal_security.py @@ -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 diff --git a/utils/security_config.py b/utils/security_config.py index ce8fb29..9623113 100644 --- a/utils/security_config.py +++ b/utils/security_config.py @@ -89,16 +89,47 @@ EXCLUDED_DIRS = { 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: path: Path to check Returns: 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: 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: return True # If we can't resolve, consider it dangerous