T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/scaffold_hjb_problem.py:213
- Finding
- Unvalidated problem slug allows arbitrary file creation outside the project directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/scaffold_hjb_problem.py`, lines 213–240 **Vulnerability Type**: Path traversal and arbitrary file creation **Risk Level**: Medium ### Vulnerable Code ```python module_slug = args.name.strip().lower() class_prefix = snake_to_camel(module_slug) control_names = [c.strip() for c in args.control_names.split(",") if c.strip()] if len(control_names) != args.num_controls: raise ValueError("num-controls must match number of control-names") # Ensure DGM framework is present; copy from bundled assets if not. repo_root = Path.cwd() bootstrap_framework(repo_root) write_file( repo_root / "src" / "configs" / f"{module_slug}_config.py", build_config(module_slug, class_prefix, args.dimension, args.num_controls, control_names), ) write_file( repo_root / "src" / "problems" / f"{module_slug}_problem.py", build_problem(class_prefix), ) write_file( repo_root / "src" / "losses" / f"{module_slug}_loss.py", build_loss(class_prefix), ) write_file( repo_root / "examples" / f"{module_slug}_train.py", build_example(module_slug, class_prefix), ) ``` The destination-writing helper also creates attacker-selected parent directories: ```python def write_file(path: Path, content: str) -> None: path.parent.mkdir(parents=True, exist_ok=True) if path.exists(): raise FileExistsError(f"File already exists: {path}") path.write_text(content, encoding="utf-8") ``` ### Technical Analysis The value supplied through `--name` is only stripped and converted to lowercase. It is not validated as a Python identifier or restricted to a safe slug format. Path separators, `..` components, and absolute path syntax can consequently become part of the destination passed to `Path`. Python path composition normalizes traversal components when the path is used by the filesystem. In addition, if a later path component is absolute, `pathlib` can discard preceding components. The generated filen ...[truncated 2285 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict problem names to a conservative slug syntax before performing any filesystem operation: ```python import re SAFE_SLUG = re.compile(r"^[a-z][a-z0-9_]*$") module_slug = args.name.strip().lower() if not SAFE_SLUG.fullmatch(module_slug): raise ValueError( "--name must begin with a letter and contain only lowercase " "letters, digits, and underscores" ) ``` 2. Resolve and verify each output path against an explicitly resolved repository root: ```python repo_root = Path.cwd().resolve() def safe_destination(relative_path: Path) -> Path: destination = (repo_root / relative_path).resolve() try: destination.relative_to(repo_root) except ValueError as exc: raise ValueError(f"Output path escapes repository root: {destination}") from exc return destination ``` 3. Build destinations exclusively from validated relative components: ```python config_path = safe_destination( Path("src") / "configs" / f"{module_slug}_config.py" ) ``` 4. Reject all absolute paths, path separators, `.` components, and `..` components even if additional validation is added elsewhere. 5. Where hostile local filesystem state is in scope, account for symbolic links and time-of-check/time-of-use races. Avoid following untrusted symlinks and use atomic, exclusive file creation rather than a separate existence check followed by `write_text()`. 6. Run scaffolding with the minimum filesystem permissions required and avoid executing it from privileged or broadly writable automation contexts. ]]>