T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/organize_downloads.py:10
- Finding
- Unvalidated Rule Group Names Permit Destination Path Escape and File Overwrite## Vulnerability Details **File Location**: `scripts/organize_downloads.py`, lines 10–34 **Vulnerability Type**: Unvalidated path construction and path traversal **Risk Level**: Medium ### Vulnerable Code ```python def detect_group(ext, groups): ext = ext.lower() for name, exts in groups.items(): if ext in exts: return name return "other" def main(): ap = argparse.ArgumentParser() ap.add_argument("folder", help="Downloads folder") ap.add_argument("--rules", default="resources/rules.sample.json") ap.add_argument("--apply", action="store_true", help="Actually move files") args = ap.parse_args() rules = load_rules(args.rules) folder = Path(args.folder) moves = [] for item in folder.iterdir(): if item.is_file(): group = detect_group(item.suffix, rules["groups"]) target_dir = folder / group / item.stat().st_mtime_ns.__str__()[:7] target = target_dir / item.name moves.append({"source": str(item), "target": str(target), "group": group}) if args.apply: target_dir.mkdir(parents=True, exist_ok=True) shutil.move(str(item), str(target)) ``` ### Technical Analysis The group name is obtained from a caller-selected JSON rules file and used directly as a filesystem path component. The implementation does not reject absolute paths, path separators, `.` or `..` components. It also does not resolve the resulting destination and verify that it remains beneath the user-selected folder. A malicious rule name such as `../../external` can therefore cause the computed destination to escape the intended Downloads directory. An existing symlink used as a group directory could produce a similar scope escape. The `--apply` branch creates the destination directories and calls `shutil.move` without checking whether the destination already exists. Depend ...[truncated 1504 chars]
- Remediation
- ## Remediation Suggestions 1. Treat every group name loaded from a rules file as untrusted input. 2. Require group names to be safe single path components. Reject absolute paths, empty names, `.` and `..`, directory separators, and platform-specific alternate separators. 3. Resolve the base folder and every proposed destination, then verify confinement before previewing or moving: ```python base = folder.resolve() destination = (base / group / date_part / item.name).resolve() if destination != base and base not in destination.parents: raise ValueError(f"Destination escapes the selected folder: {destination}") ``` 4. Do not follow attacker-controlled symlinked destination components. Validate each existing component with `lstat()` or use platform facilities that provide directory-relative, no-follow operations where available. 5. Check `target.exists()` before moving. Refuse collisions by default and require an explicit user-selected policy for renaming, skipping, or overwriting. 6. Validate the complete rules schema, including group-name and extension types, before processing any files. 7. Revalidate all destinations immediately before the apply operation rather than relying only on an earlier preview. 8. Add regression tests covering absolute group names, `../` traversal, nested separators, symlink escapes, and existing destination collisions.