T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_podcast.py:382
- Finding
- Batch Item Identifier Allows Arbitrary File Writes Outside the Output Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate_podcast.py:382-388` **Vulnerability Type**: Path traversal and unrestricted file overwrite **Risk Level**: Medium ### Vulnerable Code ```python file_id = item.get("id", f"podcast_{i:03d}") url = item.get("url") text = item.get("text") category = item.get("category", "") output_path = os.path.join(output_dir, f"{file_id}.mp3") result = await generate_podcast( url=url, text=text, output_path=output_path, timeout=timeout, on_progress=on_progress, ) ``` The resulting path is subsequently created and overwritten: ```python os.makedirs(os.path.dirname(output_path) or ".", exist_ok=True) with open(output_path, "wb") as f: for chunk in audio_chunks: f.write(chunk) ``` ### Technical Analysis The batch item `id` is read directly from an externally supplied JSON file and interpolated into a filesystem path without validation or canonicalization. `os.path.join()` does not enforce containment beneath `output_dir`. An identifier containing parent-directory components, such as `../../shared/recording`, escapes the configured output directory. On supported platforms, an absolute identifier can also cause `os.path.join()` to discard the preceding output directory entirely. The program creates missing parent directories and opens the destination using `"wb"`, which truncates an existing file before writing generated audio. The `.mp3` suffix limits the names that can be targeted but does not prevent unauthorized writes or overwrites of writable files ending in that suffix. ### Attack Path 1. An attacker creates or modifies a batch JSON file accepted through `--batch`. 2. The attacker sets an item identifier to a traversal value, for example: ```json { "id": "../../shared/recording", "text": "Attacker-selected audio content" } ``` 3. The operator runs the batch generator with an output directory such as `./podcasts`. 4. The application constr ...[truncated 870 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict `file_id` to a conservative allowlist: ```python import re SAFE_ID = re.compile(r"^[A-Za-z0-9_-]+$") if not isinstance(file_id, str) or not SAFE_ID.fullmatch(file_id): raise ValueError("Invalid batch item identifier") ``` 2. Resolve and verify the destination remains beneath the output directory: ```python from pathlib import Path base_dir = Path(output_dir).resolve() output_path = (base_dir / f"{file_id}.mp3").resolve() if output_path.parent != base_dir: raise ValueError("Output path escapes the configured directory") ``` 3. Explicitly reject absolute paths, `..` components, directory separators, control characters, and platform-specific alternate separators. 4. If replacing files is not required, open new outputs in exclusive creation mode (`"xb"`) or require explicit confirmation before overwriting an existing destination. 5. Validate the complete batch document against a schema before processing it, including identifier type, maximum length, and permitted characters. ]]>