T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/qwen_audio.py:224
- Finding
- Arbitrary File Write Through Unsanitized Output Path## Vulnerability Details **File Location**: `scripts/qwen_audio.py`, lines 224–226; vulnerable write sink at lines 134–137 **Vulnerability Type**: Path traversal and unrestricted file write **Risk Level**: High The output filename supplied through `--output` is used without validating whether the resulting path remains inside the configured audio output directory. ```python def download_file(url: str, out_path: pathlib.Path) -> pathlib.Path: out_path.parent.mkdir(parents=True, exist_ok=True) with urllib.request.urlopen(url, timeout=120) as resp: out_path.write_bytes(resp.read()) return out_path ``` ```python if args.download: ext = args.format or "wav" filename = args.output or f"{sanitize_name(args.voice)}-{int(time.time())}.{ext}" path = output_dir() / filename download_file(audio_url, path) result["file"] = str(path) ``` ### Technical Analysis `args.output` is attacker-influenced command-line input. Unlike automatically generated filenames, this value is not passed through `sanitize_name()` or subjected to path-containment validation. In Python path handling, joining a base directory with an absolute path discards the base directory. Relative paths containing `../` can also traverse outside the intended directory. Consequently, values such as `../../target.wav` or `/absolute/path/target.wav` cause `download_file()` to write outside `QWEN_AUDIO_OUTPUT_DIR`. The sink creates missing parent directories with `mkdir(parents=True, exist_ok=True)` and then uses `write_bytes()`, which truncates and overwrites an existing destination. There is no check for an existing file, symbolic link, absolute path, traversal component, or resolved-path containment. The written content is the audio response downloaded from the Qwen service, so an attacker does not necessarily control arbitrary byte-for-byte content. Nevertheless, the destination is unrestricted within the operating-sys ...[truncated 1714 chars]
- Remediation
- ## Remediation Suggestions 1. Accept only a basename for `--output`, rejecting absolute paths and directory components: ```python requested = pathlib.Path(args.output) if requested.is_absolute() or requested.name != args.output: fail("--output must be a filename without directory components.") ``` 2. Resolve and validate the final destination against the configured output directory: ```python base = output_dir().resolve() destination = (base / requested).resolve() try: destination.relative_to(base) except ValueError: fail("Output path must remain inside the configured output directory.") ``` 3. Validate containment after path resolution to prevent both `../` traversal and absolute-path bypasses. 4. Defend against symbolic-link attacks. Where feasible, reject symlink destinations and use secure file-opening semantics that prevent following symbolic links. 5. Avoid silent overwrites. Use exclusive file creation by default or require an explicit `--overwrite` option before replacing an existing file. 6. Apply the same centralized output-path validation to every command that accepts or derives an output filename, ensuring future commands cannot bypass the restriction.