T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/runninghub_video.py:268
- Finding
- Server-Controlled Task Identifier Used in Output File Path<![CDATA[ ## Vulnerability Details **File Location**: `scripts/runninghub_video.py`, lines 268-277 **Vulnerability Type**: Path traversal and arbitrary file overwrite **Risk Level**: Medium ### Vulnerable Code ```python def save_outputs(result: dict, out_dir: Path) -> list[Path]: results = result.get("results") or [] if not results: raise SystemExit( "Task finished but no results were returned:\n" f"{json.dumps(result, ensure_ascii=False, indent=2)}" ) out_dir.mkdir(parents=True, exist_ok=True) task_id = result.get("taskId", "runninghub-task") saved_paths: list[Path] = [] for index, item in enumerate(results): url = item.get("url") if not url: continue extension = guess_extension(item, index) destination = out_dir / f"{task_id}-{index + 1}.{extension}" download_result(str(url), destination) ``` The associated extension is also derived from remote response data without an allowlist: ```python def guess_extension(item: dict, index: int) -> str: output_type = item.get("outputType") if output_type: suffix = str(output_type).strip().lstrip(".") return suffix or "bin" parsed = urllib.parse.urlparse(str(item.get("url", ""))) name = Path(parsed.path).name suffix = Path(name).suffix.lstrip(".") if suffix: return suffix return "mp4" if index == 0 else "bin" ``` ### Technical Analysis The `taskId`, `outputType`, and result URL are supplied by the remote API. The code incorporates `taskId` and the derived extension directly into a local path without rejecting path separators, `..` components, control characters, or other unsafe filename content. The destination is not resolved and checked against the intended output directory before `download_result()` opens it with `"wb"`. A traversal-bearing `taskId`, such as `../../target`, can therefore make the resulting path resolve outside `out_dir`. Exploit ...[truncated 1493 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not use a remote task identifier directly as a filename. Generate a local identifier, such as a UUID. - If the task identifier must remain visible, replace every character outside a strict allowlist such as `[A-Za-z0-9_-]`. - Map output types through a fixed extension allowlist, for example: ```python ALLOWED_EXTENSIONS = { "mp4": "mp4", "webm": "webm", "png": "png", "jpg": "jpg", "jpeg": "jpg", } ``` - Resolve and validate every destination before opening it: ```python base = out_dir.resolve() destination = (base / safe_name).resolve() if destination.parent != base: raise SystemExit("Unsafe output path returned by remote service") ``` - Use exclusive creation where overwriting is unnecessary, or require explicit confirmation before replacing an existing file. - Download to a temporary file inside the validated output directory and atomically rename it only after the download succeeds. ]]>