T09 · Insecure Skill Coding Practices
- Location
scripts/download_step_part.py:119- Finding
Unrestricted Download Filename Permits Writes Outside the Output Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/download_step_part.py, lines 119-145
Vulnerability Type: Path traversal and arbitrary file write
Risk Level: Mediumpython def filename_for(part: dict[str, Any], requested_filename: str | None, allow_requested: bool) -> str: if requested_filename and allow_requested: return requested_filename step_url = str(part.get("stepUrl") or "") name = Path(urllib.parse.urlparse(step_url).path).name if name: return name return f"{part['id']}.step" def step_download_url(part: dict[str, Any], origin: str) -> str: step_url = part.get("stepUrl") if not step_url: raise SystemExit(f"Part {part.get('id', '<unknown>')} does not include stepUrl.") return urllib.parse.urljoin(f"{origin_url(origin)}/", str(step_url)) def write_download(part: dict[str, Any], args: argparse.Namespace, allow_requested_filename: bool) -> dict[str, Any]: step_url = step_download_url(part, args.origin) out_dir = Path(args.out_dir) out_dir.mkdir(parents=True, exist_ok=True) path = out_dir / filename_for(part, args.filename, allow_requested_filename) if path.exists() and not args.overwrite: raise SystemExit(f"Refusing to overwrite existing file: {path}") data = request(step_url, args.timeout) path.write_bytes(data)Technical Analysis
The value accepted through
--filenameis returned without validation. It may therefore contain an absolute path or parent-directory components such as../.With
pathlib, joining a base directory to an absolute second path discards the base directory. Relative traversal components can similarly resolve outsideargs.out_dir. The implementation does not resolve the resulting path and verify that it remains under the intended output directory before callingwrite_bytes().Existing files are protected by default, but supplying `--overw ...[truncated 1540 chars]
- Remediation
View remediation
Remediation Suggestions
- Restrict
--filenameto a basename:python requested = Path(requested_filename) if requested.is_absolute() or requested.name != requested_filename: raise SystemExit("--filename must be a plain filename without directory components.") - Resolve both paths and enforce containment before writing:
python out_dir = Path(args.out_dir).resolve() path = (out_dir / safe_filename).resolve() if path.parent != out_dir: raise SystemExit("Download path escapes the output directory.") - Reject
.and.., path separators for all supported platforms, null characters, and platform-specific absolute path forms. - Consider restricting writes to a dedicated download directory rather than accepting arbitrary output directories.
- Retain the default no-overwrite behavior and require explicit, separately confirmed authorization before replacing an existing file.
- Use exclusive temporary-file creation followed by an atomic rename after successful validation to reduce race-condition and partial-write risks.
- Restrict