T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_image.py:284
- Finding
- Path Traversal Through Server-Controlled Image Filename## Vulnerability Details **File Location**: `scripts/generate_image.py`, lines 218–229 and 284–290 **Vulnerability Type**: Path traversal and arbitrary file placement **Risk Level**: Medium ### Vulnerable Code ```python def download_image(url: str, filename: str, output_dir: Path) -> Path: """Download an image to the specified directory.""" output_path = output_dir / filename print(f"\n📥 Downloading image...") print(f" URL: {url}") print(f" Saving to: {output_path}") try: output_dir.mkdir(parents=True, exist_ok=True) request.urlretrieve(url, output_path) ``` The filename passed to this function is derived directly from a remote API response: ```python for img_data in generated_urls: url = img_data.get("url") name = img_data.get("name") if not url or not name: print(f"⚠️ Skipping invalid data: {img_data}") continue filename = f"{name}.png" path = download_image(url, filename, output_dir) ``` ### Technical Analysis The `name` field is controlled by the remote image-generation service and is concatenated with `.png` without validation. The resulting value is joined to `output_dir` using: ```python output_path = output_dir / filename ``` Python's `pathlib` does not automatically restrict the resulting path to the intended directory. A name containing parent-directory components, such as `../../target`, can resolve outside `qingping-ai/`. An absolute name can also cause the original output directory to be discarded. The `.png` suffix limits the destination to a filename ending in `.png`, but it does not prevent overwriting existing image files or placing attacker-controlled content at arbitrary writable paths with that suffix. This behavior exceeds the minimum filesystem privileges required by the Skill. Generated files only need to be written beneath the documented `qingping-ai/` directory. ### Attack ...[truncated 1206 chars]
- Remediation
- ## Remediation Suggestions Treat every filename returned by the API as untrusted. 1. Prefer generating filenames locally with a UUID or other cryptographically random identifier. 2. If the remote name must be retained, reject absolute paths, parent-directory components, path separators, null bytes, and platform-specific reserved names. 3. Resolve both the output directory and destination path and verify containment before writing. 4. Open new files using exclusive creation where overwriting is not required. 5. Apply a conservative filename-length limit. Example hardening: ```python import re import uuid output_root = output_dir.resolve() safe_name = re.sub(r"[^A-Za-z0-9._-]", "_", Path(name).name) safe_name = safe_name[:100] or uuid.uuid4().hex output_path = (output_root / f"{safe_name}.png").resolve() if output_root not in output_path.parents: raise ValueError("Invalid output filename") ``` For stronger isolation, ignore the remote name entirely: ```python output_path = output_dir.resolve() / f"{uuid.uuid4().hex}.png" ```