T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/download_weights.py:96
- Finding
- Encoded Path Traversal Allows Arbitrary File Writes## Vulnerability Details **File Location**: `scripts/download_weights.py`, lines 96–121 and 137–149 **Vulnerability Type**: Path traversal and unrestricted file write **Risk Level**: High ### Vulnerable Code ```python def resolve_url_dest( raw: str, base: str, overwrite: bool, default_subfolder: str | None ) -> tuple[str | None, str | None, bool]: """Return (url, dest_path, is_skip). url None = skip/ignore.""" url = raw.strip() if not url or url.startswith("#"): return (None, None, True) subfolder = default_subfolder if default_subfolder in SUBFOLDERS else "checkpoints" if " " in url: url, sub = url.strip().split(None, 1) sub = sub.strip().lower() if sub in SUBFOLDERS: subfolder = sub elif not default_subfolder or default_subfolder not in SUBFOLDERS: subfolder = infer_subfolder(url) base = os.path.expanduser(base) model_dir = os.path.join(base, "models", subfolder) os.makedirs(model_dir, exist_ok=True) path = urlparse(url).path name = path.rstrip("/").split("/")[-1] name = unquote(name) if name else "downloaded.safetensors" out_path = os.path.join(model_dir, name) if os.path.isfile(out_path) and not overwrite: return (None, out_path, True) return (url, out_path, False) ``` ```python def download_one_fallback( url: str, dest_path: str, overwrite: bool ) -> tuple[str, str]: """Download one file with urllib. Returns (status, path_or_message).""" if os.path.isfile(dest_path) and not overwrite: return ("skipped", dest_path) req = urllib.request.Request(url, headers={"User-Agent": "ComfyUI-Skill/1.0"}) try: with urllib.request.urlopen(req, timeout=600) as resp: with open(dest_path, "wb") as f: while True: chunk = resp.read(1 << 20) if not chunk: ...[truncated 2343 chars]
- Remediation
- ## Remediation Suggestions - Decode the URL path before extracting the final filename, then reduce it to a single safe basename. - Reject filenames containing `/`, `\`, NUL characters, `.` or `..` path components, control characters, or platform-specific separators. - Resolve the model directory and proposed destination with `pathlib.Path.resolve()`. - Verify confinement with `destination.is_relative_to(model_directory)` on supported Python versions, or use a reliable `os.path.commonpath()` comparison. - Generate a safe local filename when the remote filename is missing or invalid. - Create downloads using exclusive file creation where practical and require explicit confirmation before replacing existing files. - Write to a secure temporary file inside the validated destination directory, verify the result, and atomically rename it into place. - Apply the same validated path to both the built-in downloader and the `pget` manifest. - Add regression tests covering encoded `/` and `\` separators, encoded `..`, absolute paths, mixed encoding, and platform-specific traversal forms.