T09 · Insecure Skill Coding Practices
- Location
scripts/download_weights.py:96- Finding
Encoded Path Traversal Allows Arbitrary File Writes
- Content
View full analysis
Vulnerability Details
File Location:
scripts/download_weights.py, lines 96–121 and 137–149
Vulnerability Type: Path traversal and unrestricted file write
Risk Level: HighVulnerable 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
View 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 reliableos.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
pgetmanifest. - Add regression tests covering encoded
/and\separators, encoded.., absolute paths, mixed encoding, and platform-specific traversal forms.
