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