T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_slide_images.py:108
- Finding
- Path Traversal Through Unvalidated Slide Names<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate_slide_images.py`, lines 108-129 **Vulnerability Type**: Unrestricted filesystem path construction **Risk Level**: Medium ### Vulnerable Code ```python for i, slide in enumerate(slides): name = slide["name"] prompt = slide["prompt"] print(f"[{i + 1}/{total}] {name}...") result = None for attempt in range(1, args.retries + 1): result = generate_fn(prompt, args.api_key, args.model) if result: break if attempt < args.retries: wait = args.delay * attempt * 2 print(f" Retry {attempt}/{args.retries} in {wait}s...") time.sleep(wait) if result: img_data, ext = result outpath = os.path.join(args.output_dir, f"{name}.{ext}") with open(outpath, "wb") as f: f.write(img_data) ``` ### Technical Analysis The `name` value is read directly from the user-supplied prompts JSON file and incorporated into an output path without validation. The code does not reject absolute paths, directory separators, or parent-directory components such as `../`. For a relative traversal value, `os.path.join()` produces a path that can resolve outside `args.output_dir`. If `name` is absolute, `os.path.join()` discards the preceding output directory entirely. The script then opens the resulting path in `wb` mode, which creates the file or truncates an existing file. The file extension is derived from the API response, and the content is generated image data. These constraints reduce the likelihood of directly replacing an arbitrary extension-sensitive configuration file, but they do not prevent creation or overwriting of unintended files with an image extension. ### Attack Path 1. An attacker supplies or modifies the JSON file passed through `--prompts-file`. 2. The attacker sets a slide name to a traversal or absolute path, such as: ```json { "name": "../../shared/cover", ...[truncated 1233 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Treat each slide name as a filename rather than a path: 1. Reject empty names, absolute paths, `.` or `..` components, and all directory separators. 2. Apply a conservative allowlist, such as letters, digits, underscores, and hyphens. 3. Resolve both the output directory and candidate output path and verify that the candidate remains beneath the output directory. 4. Consider using exclusive creation mode if overwriting existing files is unnecessary. 5. Validate the prompts JSON schema before processing any slides. Example hardening: ```python import re from pathlib import Path SAFE_NAME = re.compile(r"^[A-Za-z0-9_-]{1,100}$") output_dir = Path(args.output_dir).resolve() output_dir.mkdir(parents=True, exist_ok=True) name = slide.get("name") if not isinstance(name, str) or not SAFE_NAME.fullmatch(name): raise ValueError(f"Invalid slide name: {name!r}") outpath = (output_dir / f"{name}.{ext}").resolve() if output_dir not in outpath.parents: raise ValueError("Output path escapes the requested output directory") with outpath.open("wb") as f: f.write(img_data) ``` The response-derived extension should also be checked against an explicit allowlist such as `{"png", "jpg", "jpeg", "webp"}`. ]]>