T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/find_stl.py:187
- Finding
- Path Traversal Through Untrusted Remote Model Filename## Vulnerability Details **File Location**: `scripts/find_stl.py`, lines 187-191; file-write sink at lines 111-120 **Vulnerability Type**: Path traversal leading to arbitrary file creation or overwrite **Risk Level**: High ### Vulnerable Code ```python for fobj in stls: fid = str(fobj["id"]) name = fobj.get("name") or f"file-{fid}" link = printables_get_download_link(p["id"], "stl", [fid]) out_path = os.path.join(base_dir, "files", name) download_file(link, out_path) ``` The resulting path reaches the following file-write sink: ```python def download_file(url: str, out_path: str, timeout: int = 60) -> None: os.makedirs(os.path.dirname(out_path), exist_ok=True) req = urllib.request.Request(url, headers={"user-agent": "Mozilla/5.0"}) with urllib.request.urlopen(req, timeout=timeout) as r: with open(out_path, "wb") as f: while True: b = r.read(1024 * 1024) if not b: break f.write(b) ``` ### Technical Analysis The `name` value is obtained from remotely supplied Printables model metadata and is passed directly to `os.path.join()` without filename sanitization, path normalization, or destination containment validation. `os.path.join(base_dir, "files", name)` does not guarantee that the result remains inside the intended download directory. A filename containing parent-directory components such as `../../target` can escape that directory. On supported platforms, an absolute filename can also cause the preceding path components to be discarded. The calculated path is passed to `download_file()`, which creates parent directories and opens the destination in `wb` mode. Consequently, an escaped destination is created or overwritten without confirmation. The `safe_slug()` protection applied to the model directory does not protect individual remote filenames. ### Attack Path 1. An attac ...[truncated 1628 chars]
- Remediation
- ## Remediation Suggestions 1. Treat every remote filename as untrusted. Reject absolute paths, parent-directory components, path separators, NUL characters, and platform-specific drive or UNC path syntax. 2. Reduce the remote value to a safe filename using a strict allowlist or a sanitized basename. Generate a local filename from the trusted file ID when the supplied name is invalid. 3. Resolve both the destination root and candidate path to canonical absolute paths, then verify containment before creating directories or opening the file. 4. Refuse duplicate destinations and existing files by default. Require an explicit overwrite option if replacement is intended. 5. Open newly downloaded files with exclusive creation mode where practical to reduce unintended overwrites and race conditions. 6. Add tests covering `../`, nested traversal, absolute paths, Windows drive paths, UNC paths, mixed separators, empty names, and duplicate sanitized names. Example containment pattern: ```python files_dir = os.path.realpath(os.path.join(base_dir, "files")) os.makedirs(files_dir, exist_ok=True) remote_name = fobj.get("name") or f"file-{fid}" safe_name = os.path.basename(remote_name.replace("\\", "/")) if not safe_name or safe_name in {".", ".."}: safe_name = f"file-{fid}" out_path = os.path.realpath(os.path.join(files_dir, safe_name)) if os.path.commonpath([files_dir, out_path]) != files_dir: raise RuntimeError(f"Unsafe remote filename: {remote_name!r}") if os.path.exists(out_path): raise RuntimeError(f"Refusing to overwrite existing file: {out_path}") download_file(link, out_path) ``` For stronger isolation, derive the local filename entirely from the trusted file ID and retain the original remote filename only as metadata in `manifest.json`.