diff --git a/tools/file_operations.py b/tools/file_operations.py index 96bdc2d53..bcf27118f 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -112,6 +112,81 @@ def _is_write_denied(path: str) -> bool: return False +# SECURITY: Path traversal detection patterns +_PATH_TRAVERSAL_PATTERNS = [ + re.compile(r'\.\./'), # Unix-style traversal + re.compile(r'\.\.\\'), # Windows-style traversal + re.compile(r'\.\.$'), # Bare .. at end + re.compile(r'%2e%2e[/\\]', re.IGNORECASE), # URL-encoded traversal + re.compile(r'\.\.//'), # Double-slash traversal + re.compile(r'^/~'), # Attempted home dir escape via tilde +] + + +def _contains_path_traversal(path: str) -> bool: + """Check if path contains directory traversal attempts. + + SECURITY FIX (V-002): Detects path traversal patterns like: + - ../../../etc/passwd + - ..\\..\\windows\\system32 + - %2e%2e%2f (URL-encoded) + - ~/../../../etc/shadow (via tilde expansion) + """ + if not path: + return False + + # Check against all traversal patterns + for pattern in _PATH_TRAVERSAL_PATTERNS: + if pattern.search(path): + return True + + # Check for null byte injection (CWE-73) + if '\x00' in path: + return True + + # Check for overly long paths that might bypass filters + if len(path) > 4096: + return True + + return False + + +def _validate_safe_path(path: str, operation: str = "access") -> tuple[bool, str]: + """Validate that a path is safe for file operations. + + Returns: + (is_safe, error_message) tuple. If is_safe is False, error_message + contains the reason. + + SECURITY FIX (V-002): Centralized path validation to prevent: + - Path traversal attacks (../../../etc/shadow) + - Home directory expansion attacks (~user/malicious) + - Null byte injection + """ + if not path: + return False, "Path cannot be empty" + + # Check for path traversal attempts + if _contains_path_traversal(path): + return False, ( + f"Path traversal detected in '{path}'. " + f"Access to paths outside the working directory is not permitted." + ) + + # Validate path characters (prevent shell injection via special chars) + # Allow alphanumeric, spaces, common path chars, but block control chars + invalid_chars = set() + for char in path: + if ord(char) < 32 and char not in '\t\n': # Control chars except tab/newline + invalid_chars.add(repr(char)) + if invalid_chars: + return False, ( + f"Path contains invalid control characters: {', '.join(invalid_chars)}" + ) + + return True, "" + + # ============================================================================= # Result Data Classes # ============================================================================= @@ -475,6 +550,11 @@ class ShellFileOperations(FileOperations): Returns: ReadResult with content, metadata, or error info """ + # SECURITY FIX (V-002): Validate path before any operations + is_safe, error_msg = _validate_safe_path(path, "read") + if not is_safe: + return ReadResult(error=f"Security violation: {error_msg}") + # Expand ~ and other shell paths path = self._expand_path(path) @@ -663,6 +743,11 @@ class ShellFileOperations(FileOperations): Returns: WriteResult with bytes written or error """ + # SECURITY FIX (V-002): Validate path before any operations + is_safe, error_msg = _validate_safe_path(path, "write") + if not is_safe: + return WriteResult(error=f"Security violation: {error_msg}") + # Expand ~ and other shell paths path = self._expand_path(path)