T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/render_preview.py:114- Finding
Path Traversal Enables Arbitrary PlantUML File Disclosure and SVG Overwrite
- Content
View full analysis
bool: src_path = os.path.join(SRC_DIR, name + ".puml") with open(src_path, encoding="utf-8") as fp: source = fp.read() status, data = fetch(source, fmt) if status == 200 and is_real_diagram(data): os.makedirs(OUT_DIR, exist_ok=True) dst = os.path.join(OUT_DIR, name + ".svg") with open(dst, "wb") as fp: fp.write(data) print(" [OK] %-20s %6d bytes" % (name, len(data))) return True ``` The value used as `name` comes directly from command-line arguments: ```python def main(): args = [a for a in sys.argv[1:] if not a.startswith("--")] only_check = "--check" in sys.argv all_names = sorted( os.path.splitext(os.path.basename(p))[0] for p in glob.glob(os.path.join(SRC_DIR, "*.puml")) ) names = args if args else all_names ``` ### Technical Analysis The script treats each command-line argument as a diagram name, but it does not restrict that value to a filename or verify that the resulting path remains inside the expected directory. `os.path.join(SRC_DIR, name + ".puml")` does not neutralize `../` components or absolute paths. Consequently, a crafted name can resolve outside `examples/`. If a matching readable file exists, the script reads its complete contents and passes them to `fetch()`, which transmits the source to the configured remote PlantUML server. The same untrusted value is used to construct the output path: ```python dst = os.path.join(OUT_DIR, name + ".svg") ``` A traversal value can therefore direct the returned SVG outside `assets/preview`. If the parent directory exists and the process has permis ...[truncated 1495 chars]- Remediation
View remediation
str: if not NAME_RE.fullmatch(name): raise ValueError("Invalid diagram name") return name ``` 2. Resolve and verify source and destination containment before any file operation: ```python from pathlib import Path SRC_ROOT = Path(SRC_DIR).resolve() OUT_ROOT = Path(OUT_DIR).resolve() def contained_path(root: Path, name: str, suffix: str) -> Path: validate_name(name) candidate = (root / f"{name}{suffix}").resolve() if candidate.parent != root: raise ValueError("Path escapes the permitted directory") return candidate ``` 3. Reject absolute paths, path separators, `.` components, and `..` components even if a broader naming policy is later adopted. 4. Derive selectable names from the enumerated `examples/*.puml` files and reject any requested name not present in that allowlist. 5. Use atomic output creation and avoid overwriting existing files unexpectedly. For example, write to a temporary file inside `OUT_DIR` and atomically replace only the intended preview. 6. Add regression tests covering `../`, absolute paths, nested traversal, platform-specific separators, and valid diagram basenames. ]]>