T09 · Insecure Skill Coding Practices
- Location
scripts/organize_downloads.py:10- Finding
Unvalidated Rule Group Names Permit Destination Path Escape and File Overwrite
- Content
View full analysis
Vulnerability Details
File Location:
scripts/organize_downloads.py, lines 10–34
Vulnerability Type: Unvalidated path construction and path traversal
Risk Level: MediumVulnerable 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
../../externalcan 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
--applybranch creates the destination directories and callsshutil.movewithout checking whether the destination already exists. Depend ...[truncated 1504 chars]- Remediation
View remediation
Remediation Suggestions
-
Treat every group name loaded from a rules file as untrusted input.
-
Require group names to be safe single path components. Reject absolute paths, empty names,
.and.., directory separators, and platform-specific alternate separators. -
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}") -
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. -
Check
target.exists()before moving. Refuse collisions by default and require an explicit user-selected policy for renaming, skipping, or overwriting. -
Validate the complete rules schema, including group-name and extension types, before processing any files.
-
Revalidate all destinations immediately before the apply operation rather than relying only on an earlier preview.
-
Add regression tests covering absolute group names,
../traversal, nested separators, symlink escapes, and existing destination collisions.
-