T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/generate.py:256
- Finding
- User-Controlled Filenames Permit Path Traversal and File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate.py`, lines 256–257, 327–340, 355–356, 369–372, and 381–382 **Vulnerability Type**: Path traversal and unsafe file creation **Risk Level**: High ### Vulnerable Code ```python def write_file(content, filepath): """Write content to file, creating directories if needed.""" os.makedirs(os.path.dirname(filepath), exist_ok=True) with open(filepath, "w", encoding="utf-8") as f: f.write(content) print(f" [OK] Created: {filepath}") ``` The vulnerable function receives paths constructed directly from command-line input: ```python filename = f"{args.title.replace(' ', '-')}-Notes.md" write_file(content, os.path.join(output_dir, filename)) ``` ```python atomic_filename = f"{concept.replace(' ', '-')}.md" write_file(atomic_content, os.path.join(output_dir, atomic_filename)) ``` ```python filename = f"{args.concept.replace(' ', '-')}.md" write_file(content, os.path.join(output_dir, filename)) ``` ```python if args.term_cn: filename = f"{args.term_en}（{args.term_cn}）.md" else: filename = f"{args.term_en}.md" write_file(content, os.path.join(output_dir, filename)) ``` ```python filename = f"{args.course.replace(' ', '-')}-MOC.md" write_file(content, os.path.join(output_dir, filename)) ``` ### Technical Analysis The `--title`, `--concepts`, `--concept`, `--term-en`, `--term-cn`, and `--course` arguments are incorporated into filesystem paths without rejecting path separators, absolute paths, or `..` traversal components. Replacing spaces with hyphens does not make a filename safe. `os.path.join(output_dir, filename)` does not guarantee that the resulting path remains inside `output_dir`. A filename containing traversal components can resolve outside the selected directory. If the generated component is absolute, `os.path.join` can discard the output-directory prefix entirely. The final path is then passed to: ```python os.makedirs(os.path.dirname(filepath), exist_ ...[truncated 2070 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Apply a strict filename policy to every user-controlled filename component: - Reject `/`, `\`, null bytes, and platform-specific separators. - Reject absolute paths. - Reject `.` and `..` path components. - Permit only a conservative set of Unicode letters, numbers, spaces, hyphens, underscores, and parentheses. - Enforce a reasonable maximum filename length. 2. Resolve and validate the destination before writing: ```python from pathlib import Path def safe_destination(output_dir, filename): base = Path(output_dir).expanduser().resolve() destination = (base / filename).resolve() if base not in destination.parents: raise ValueError("Output path escapes the configured output directory") return destination ``` 3. Use a dedicated slugification function instead of only replacing spaces: ```python def safe_slug(value): value = value.strip().replace(" ", "-") if not value or value in {".", ".."}: raise ValueError("Invalid filename") if "/" in value or "\\" in value: raise ValueError("Path separators are not allowed") return value ``` 4. Perform containment validation after appending the required suffix, not before it. 5. Avoid silent overwrite: - Use exclusive creation mode (`"x"`) by default. - Add an explicit `--overwrite` option when replacement is intentional. - Refuse to follow symbolic links where supported. 6. Add tests covering: - `../` traversal. - Absolute paths. - Backslash traversal on Windows. - Nested separators in glossary terms. - Existing-file overwrite. - Symlink-based escape attempts. ]]>