T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/tencent_hotsearch.py:295- Finding
Output Path Validation Allows Arbitrary File Overwrite Outside the Configured Directory
- Content
View full analysis
Path: """ Validate and sanitize output path to prevent directory traversal attacks. Args: output_path: Path to validate Returns: Validated Path object Raises: ValueError: If path is invalid or attempts directory traversal """ output_file = Path(output_path).resolve() # Get the configured output directory config_output_dir = Path(self.config.get('output_dir', './output')).resolve() # Ensure the output file is within the configured output directory try: output_file.relative_to(config_output_dir) except ValueError: # If output_file is not relative to config_output_dir, check if it's an absolute path # For absolute paths, just ensure it doesn't contain suspicious patterns if '..' in str(output_path): raise ValueError( f"Invalid output path: Directory traversal detected. " f"Path must be within the configured output directory ({config_output_dir})" ) # Check for suspicious patterns if '..' in str(output_path): raise ValueError( f"Invalid output path: Directory traversal not allowed. " f"Use paths within {config_output_dir}" ) # Ensure parent directory exists output_file.parent.mkdir(parents=True, exist_ok=True) return output_file ``` The validated path is subsequently opened in write mode by `save_results()`, such as: ```python with open(output_file, 'w', encoding='utf-8') as f: ``` ### Technical Analysis The validation attempts to establish that the resolved output path is contained within the configured output directory by calling `output_file ...[truncated 2198 chars]- Remediation
View remediation
Path: config_output_dir = Path( self.config.get("output_dir", "./output") ).expanduser().resolve() candidate = Path(output_path).expanduser() if not candidate.is_absolute(): candidate = config_output_dir / candidate output_file = candidate.resolve() try: output_file.relative_to(config_output_dir) except ValueError as exc: raise ValueError( f"Output path must be within {config_output_dir}" ) from exc config_output_dir.mkdir(parents=True, exist_ok=True) output_file.parent.mkdir(parents=True, exist_ok=True) return output_file ``` Additional hardening should include: 1. Treat relative output paths as relative to `output_dir`, rather than the process working directory. 2. Reject every resolved path outside `output_dir`, regardless of whether its text contains `..`. 3. Consider refusing symbolic-link targets or opening files using directory-relative, no-follow operating-system APIs where available if hostile local users are in scope. 4. Avoid overwriting existing files unless the user explicitly requests it, for example by using exclusive creation mode. 5. If arbitrary absolute paths are an intended feature, remove the false containment claim and require explicit trusted-user authorization before writing outside `output_dir`. 6. Run the Skill under a dedicated, non-privileged account with access only to the intended output directory. ]]>