T09 · Insecure Skill Coding Practices
- Location
style_fingerprint.py:263- Finding
Path Traversal Through Unsanitized Fingerprint Names
- Content
View full analysis
Optional[Dict]: """Load fingerprint.""" filepath = FINGERPRINTS_DIR / f"{name}.json" if not filepath.exists(): return None with open(filepath, 'r', encoding='utf-8') as f: return json.load(f) @staticmethod def list_all() -> List[str]: """List all fingerprints.""" files = FINGERPRINTS_DIR.glob("*.json") return sorted([f.stem for f in files]) @staticmethod def delete(name: str) -> bool: """Delete fingerprint.""" filepath = FINGERPRINTS_DIR / f"{name}.json" if filepath.exists(): filepath.unlink() return True return False ``` ### Technical Analysis The `name` argument originates from the user-controlled `--name` command-line option and is interpolated directly into a filesystem path: ```python filepath = FINGERPRINTS_DIR / f"{name}.json" ``` The implementation neither restricts the permitted characters in a fingerprint name nor resolves and validates the resulting path ag ...[truncated 2432 chars]- Remediation
View remediation
str: if not SAFE_NAME.fullmatch(name): raise ValueError( "Invalid fingerprint name: use only letters, digits, underscores, and hyphens" ) return name ``` 2. Resolve the final path and verify that it remains inside the fingerprint directory before every read, write, or deletion: ```python def fingerprint_path(name: str) -> Path: validate_name(name) base = FINGERPRINTS_DIR.resolve() candidate = (base / f"{name}.json").resolve() if candidate.parent != base: raise ValueError("Fingerprint path escapes the storage directory") return candidate ``` 3. Replace every direct construction of `filepath` in `save()`, `load()`, and `delete()` with the centralized `fingerprint_path()` function. 4. Reject absolute paths, directory separators, `.` and `..` components explicitly, even if an allowlist is implemented, as defense in depth. 5. Consider preventing silent overwrites by opening new fingerprints in exclusive creation mode (`'x'`) or requiring an explicit overwrite option. 6. Add automated tests covering traversal and absolute-path inputs, including: ```text ../target ../../target /tmp/target ..\target nested/name ``` 7. Keep filesystem permissions on the skill process limited so that a future path-validation regression cannot modify sensitive system or user files. ]]>
