T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/pipeline.py:443
- Finding
- Arbitrary File Deletion Through Untrusted Metadata Paths<![CDATA[ ## Vulnerability Details **File Location**: `scripts/pipeline.py:443-461`, with attacker-controlled metadata loaded at `scripts/pipeline.py:631-637` **Vulnerability Type**: Improper validation of file paths used for deletion **Risk Level**: High ### Vulnerable Code ```python def cleanup_from_meta(meta: Dict[str, Any], mode: str = "temp") -> Dict[str, Any]: deleted: List[str] = [] kept: List[str] = [] for key in ("wav_path", "transcript_path"): path_value = meta.get(key) if not path_value: continue path = Path(path_value) if mode in {"temp", "all"} and path.exists(): path.unlink() deleted.append(str(path)) elif path.exists(): kept.append(str(path)) video_value = meta.get("local_file") if video_value: video_path = Path(video_value) if mode == "all" and video_path.exists(): video_path.unlink() deleted.append(str(video_path)) elif video_path.exists(): kept.append(str(video_path)) ``` The affected cleanup command loads the paths directly from a user-selected metadata file: ```python def cmd_cleanup(args: argparse.Namespace) -> None: meta = load_metadata(Path(args.metadata)) mode = "all" if args.delete_video else args.mode result = cleanup_from_meta(meta, mode) meta["cleanup"] = result meta["state"] = "cleaned" if mode != "none" else meta.get("state", "prepared") save_metadata(meta, meta.get("bvid", "metadata"), Path(meta["metadata_path"])) print(json.dumps(result, ensure_ascii=False, indent=2)) ``` ### Technical Analysis The cleanup implementation treats the `wav_path`, `transcript_path`, and `local_file` properties in the metadata JSON as trusted filesystem paths. It does not canonicalize these paths or verify that they are located beneath the intended `TEMP_DIR` and `DOWNLOAD_DIR` directories. Because the `--metadata` option accepts an arbitrary JSON fil ...[truncated 1831 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Canonicalize every candidate with `Path.resolve(strict=True)` before deletion. 2. Permit deletion only when the resolved target is beneath an explicitly approved root: - `TEMP_DIR` for WAV, transcript, and segment artifacts - `DOWNLOAD_DIR` for downloaded video files 3. Reject absolute or traversal-based metadata paths that resolve outside those roots. 4. Validate that the target has the expected file type and naming pattern, such as a recognized BVID and the expected extension. 5. Do not trust `metadata_path` stored inside the metadata document. Continue writing to the validated path supplied through `--metadata`, or require metadata files to reside in a dedicated state directory. 6. Use a strict metadata schema and reject unknown, missing, or malformed fields. 7. Consider recording artifact identifiers or paths relative to trusted roots rather than storing unrestricted absolute paths. 8. Before destructive cleanup, display the resolved deletion targets and require explicit confirmation when invoked interactively. 9. Add security tests covering absolute paths, `..` traversal, paths outside approved roots, and maliciously modified metadata. A suitable containment check should follow this model: ```python def require_path_beneath(candidate: Path, root: Path) -> Path: resolved = candidate.resolve(strict=True) trusted_root = root.resolve(strict=True) if not resolved.is_relative_to(trusted_root): raise RuntimeError(f"Refusing to delete out-of-scope path: {resolved}") if not resolved.is_file(): raise RuntimeError(f"Refusing to delete non-file target: {resolved}") return resolved ``` ]]>