T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/run_task.py:78
- Finding
- Configuration-Controlled Path Traversal Enables File Movement Outside the Intended Destination## Vulnerability Details **File Location**: `scripts/run_task.py`, lines 78-96 **Vulnerability Type**: Unvalidated destination path / path traversal **Risk Level**: High ### Vulnerable Code ```python for file in source.iterdir(): if file.is_file(): ext = file.suffix.lower() target_folder = None for rule in rules: if rule.get('extension', '').lower() == ext: target_folder = rule.get('folder', 'Other') break if target_folder is None: target_folder = 'Other' target_path = destination / target_folder if self.dry_run: self.log(f"Would move: {file.name} -> {target_folder}/") else: target_path.mkdir(parents=True, exist_ok=True) shutil.move(str(file), str(target_path / file.name)) ``` ### Technical Analysis The `folder` property is read directly from task configuration and appended to the configured destination without validation or containment checking. Python path composition does not guarantee that the resulting path remains under `destination`. A value containing parent-directory components, such as `../../target`, traverses outside the intended destination. An absolute `folder` path can replace the destination entirely when the paths are combined. The code then creates the attacker-selected directory and moves every matching regular file from the source directory into it. This violates least privilege because a file-organization task should only modify files within its explicitly configured source and destination boundaries. The implementation instead allows configuration authors to select any filesystem destination writable by the process. ### Attack Path 1. An attacker supplies or modifies a file-organizer configuration accessible to the user or automation system. 2. The attacker adds a matching rule conta ...[truncated 1839 chars]
- Remediation
- ## Remediation Suggestions Treat all paths and folder names from configuration as untrusted input. 1. Reject absolute values for `target_folder`. 2. Resolve both the destination root and candidate target path before performing any filesystem operation. 3. Require the resolved candidate path to remain strictly beneath the resolved destination root. 4. Reject `..`, empty folder names, and unexpected path separators if each rule is intended to specify only one directory name. 5. Define an explicit collision policy rather than allowing platform-dependent overwrites. 6. Validate the source and destination against an approved workspace boundary when the skill runs in an agent environment. 7. Perform validation in dry-run mode as well, so unsafe configurations are reported before live execution. Example hardening: ```python destination_root = destination.resolve() folder_value = rule.get("folder", "Other") if not isinstance(folder_value, str) or not folder_value.strip(): raise ValueError("Rule folder must be a non-empty string") folder_path = Path(folder_value) if folder_path.is_absolute() or ".." in folder_path.parts: raise ValueError(f"Unsafe target folder: {folder_value}") target_path = (destination_root / folder_path).resolve() if not target_path.is_relative_to(destination_root): raise ValueError(f"Target escapes destination: {folder_value}") target_file = target_path / file.name if target_file.exists(): raise FileExistsError(f"Refusing to overwrite: {target_file}") target_path.mkdir(parents=True, exist_ok=True) shutil.move(str(file), str(target_file)) ``` For Python versions without `Path.is_relative_to()`, use `os.path.commonpath()` with resolved paths and verify that the common path equals the approved destination root.