T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/generate_worksheet.py:405
- Finding
- Arbitrary File Read and Write Through Unvalidated Worksheet Paths## Vulnerability Details **File Location**: `scripts/generate_worksheet.py`, lines 405–425 **Vulnerability Type**: Path traversal and unrestricted filesystem access **Risk Level**: High ### Vulnerable Code ```python out_dir = spec_path.parent worksheet_file = spec.get("worksheet_file", "worksheet.html") answer_key_file = spec.get("answer_key_file", "answer-key.md") pdf_file = args.pdf_file or spec.get("pdf_file", "worksheet.pdf") spec["worksheet_file"] = worksheet_file template_path = ROOT / spec.get("template", str(DEFAULT_TEMPLATE.relative_to(ROOT))) html_text, answers, count = render_html(spec, template_path) answer_key = render_answer_key(spec, answers, count) worksheet_path = out_dir / worksheet_file answer_key_path = out_dir / answer_key_file worksheet_path.write_text(html_text, encoding="utf-8") answer_key_path.write_text(answer_key, encoding="utf-8") print(f"generated: {worksheet_path}") print(f"generated: {answer_key_path}") print(f"items: {count}") page_config = spec.get("page", {}) verify_requested = args.verify_print or page_config.get("verify_print") pdf_requested = not args.no_pdf or args.pdf or args.pdf_file or spec.get("pdf_file") or page_config.get("pdf") pdf_required = args.pdf or args.pdf_file or spec.get("pdf_file") or page_config.get("pdf_required") or verify_requested pdf_path = out_dir / pdf_file ``` ### Technical Analysis The worksheet generator accepts `template`, `worksheet_file`, `answer_key_file`, and `pdf_file` values from a worksheet JSON specification without validating that the resulting paths remain inside approved directories. Python path joining does not provide containment. If the second operand is absolute, it replaces the intended base path. Relative values containing `../` can also traverse outside `out_dir` or `ROOT`. Consequently: - `template` can select an arbitrary readable text file and cause it to be loaded by `render_html()`. - `worksheet_file` ...[truncated 2103 chars]
- Remediation
- ## Remediation Suggestions 1. Treat output fields as filenames rather than arbitrary paths. Reject absolute paths, `..` components, path separators, empty names, and unsupported extensions. 2. Resolve every destination and verify containment before access: ```python def contained_path(root: Path, supplied: str, allowed_suffixes: set[str]) -> Path: candidate_value = Path(supplied) if candidate_value.is_absolute() or ".." in candidate_value.parts: raise ValueError("Absolute paths and parent traversal are not allowed") candidate = (root / candidate_value).resolve() resolved_root = root.resolve() candidate.relative_to(resolved_root) if candidate.suffix.lower() not in allowed_suffixes: raise ValueError("Unsupported output extension") return candidate ``` 3. Restrict templates to an explicit allowlist under `assets/worksheet/`. Do not allow worksheet specifications to select arbitrary local files. 4. Use fixed output names such as `worksheet.html`, `answer-key.md`, and `worksheet.pdf` unless custom names are essential. 5. Account for symlink escapes by validating resolved paths immediately before each read or write. 6. Refuse to overwrite existing files by default. Require an explicit, trusted overwrite option when replacement is intended. 7. Use atomic writes through a temporary file in the validated destination directory followed by `os.replace()`. 8. Apply the same validation to `--pdf-file`, since command-line input currently reaches the vulnerable path construction. 9. Add regression tests covering absolute paths, nested traversal, symlink traversal, invalid extensions, and existing-file overwrite attempts.