T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_report.py:536
- Finding
- Unrestricted Arbitrary File Overwrite Through the Output Path## Vulnerability Details **File Location**: `scripts/generate_report.py`, lines 536–538; file writes occur at lines 544–546, 551–553, and 562–564 **Vulnerability Type**: Arbitrary file overwrite due to an unrestricted output path **Risk Level**: Medium **Vulnerable code:** ```python if "--output" in args: idx = args.index("--output") output_path = args[idx + 1] args = [a for i, a in enumerate(args) if i not in (idx, idx+1)] ``` The attacker-controlled path is subsequently used by all report-generation modes: ```python if mode == "--index": html = make_full_index() with open(output_path, 'w', encoding='utf-8') as f: f.write(html) ``` ```python elif mode == "--detail": if topic in KNOWLEDGE_DATA: html = make_detail_report(topic) with open(output_path, 'w', encoding='utf-8') as f: f.write(html) ``` ```python elif mode == "--general": if topic in GENERAL_TOPICS: html = make_general_report(topic) with open(output_path, 'w', encoding='utf-8') as f: f.write(html) ``` ### Technical Analysis The undocumented `--output` argument accepts an arbitrary absolute or relative filesystem path. The code performs no path normalization, directory confinement, filename-extension validation, symlink rejection, or overwrite confirmation. The selected path is passed directly to `open()` with mode `'w'`. If the target already exists, Python truncates it before writing the generated HTML. Consequently, a caller who can influence command-line arguments can replace any file writable by the process. This behavior exceeds the documented workflow in `SKILL.md`, which states that the report is written to `woodworking_report.html` in the current working directory. Although the generated content is static woodworking HTML and the code does not elevate privileges, the unrestricted destination creates a destructive local-file pr ...[truncated 1620 chars]
- Remediation
- ## Remediation Suggestions 1. Remove `--output` if custom destinations are not required and always write to a fixed report filename in an approved directory. 2. If custom filenames are required, accept only a basename rather than a complete path. 3. Resolve the destination using `pathlib.Path.resolve()` and verify that it remains inside a dedicated report directory. 4. Reject absolute paths, `..` traversal components, non-HTML extensions, and directory targets. 5. Reject symlink destinations or open files using platform-appropriate no-follow protections. 6. Avoid silent truncation. Use exclusive creation mode (`'x'`) by default or require explicit overwrite confirmation. 7. Handle a missing value after `--output` and report a controlled argument-validation error. 8. Prefer `argparse` for robust command-line parsing and validation. 9. Add tests covering absolute paths, traversal attempts, symlinks, existing files, missing option values, and destinations outside the approved directory. Example confinement approach: ```python from pathlib import Path REPORT_DIR = (Path.cwd() / "reports").resolve() REPORT_DIR.mkdir(parents=True, exist_ok=True) requested_name = Path(user_value) if requested_name.is_absolute() or requested_name.name != user_value: raise ValueError("Output must be a filename without directory components") if requested_name.suffix.lower() != ".html": raise ValueError("Output filename must use the .html extension") output_path = (REPORT_DIR / requested_name.name).resolve() if output_path.parent != REPORT_DIR: raise ValueError("Output path escapes the report directory") with output_path.open("x", encoding="utf-8") as report_file: report_file.write(html) ```