T09 · Insecure Skill Coding Practices
Warning
- Location
tools/check.py:34- Finding
Path Traversal Through Unvalidated Preset Names
- Content
View full analysis
Path: """Get history file path for a specific preset.""" return RUNTIME_DIR / f"history_{preset_name}.json" def load_history(preset_name: str) -> dict[str, dict[str, object]]: """Load previous check results for a preset.""" path = get_history_path(preset_name) if path.exists(): with path.open("r", encoding="utf-8") as f: return json.load(f) return {} def save_history(preset_name: str, current: dict[str, dict[str, object]]) -> None: """Save current results as history for next comparison.""" path = Path(get_history_path(preset_name)) path.parent.mkdir(parents=True, exist_ok=True) with path.open("w", encoding="utf-8") as f: json.dump(current, f, ensure_ascii=False, indent=2) ``` The preset name is accepted when it exists as a key in the editable preset configuration: ```python presets = load_presets() if preset_name not in presets: print(json.dumps({ "error": f"Preset '{preset_name}' not found", "available": list(presets.keys()), }, ensure_ascii=False)) return 1 preset = presets[preset_name] ``` The resulting history is subsequently written using that name: ```python save_history(preset_name, current_state) ``` ### Technical Analysis `get_history_path()` interpolates `preset_name` directly into a filesystem path. It does not restrict path separators, traversal components such as `..`, absolute path syntax, or the final resolved location. The check that the supplied name exists in `presets.json` is not a sufficient security boundary. The project explicitly documents preset configuration as editable, and JSON object keys can contain path separators and ...[truncated 2138 chars]- Remediation
View remediation
None: if not PRESET_NAME_PATTERN.fullmatch(name): raise ValueError("Preset name contains unsupported characters") ``` 2. Resolve the generated path and verify that it remains beneath `RUNTIME_DIR`: ```python def get_history_path(preset_name: str) -> Path: validate_preset_name(preset_name) runtime_root = RUNTIME_DIR.resolve() path = (runtime_root / f"history_{preset_name}.json").resolve() if not path.is_relative_to(runtime_root): raise ValueError("History path escapes the runtime directory") return path ``` 3. For compatibility with Python versions lacking `Path.is_relative_to()`, use `relative_to()` and reject `ValueError`. 4. Consider deriving filenames from a safe identifier or a cryptographic hash rather than using configuration keys directly. 5. Reject absolute names, path separators, `.` and `..` components, control characters, and platform-specific separator variants. 6. Add tests for traversal payloads, including repeated `../`, backslashes on Windows, absolute paths, and encoded or unusual separator characters. 7. If concurrent execution is possible, write history atomically through a temporary file inside `runtime`, then replace the intended history file. ]]>