T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/skill_sync.py:1193- Finding
Path Traversal and Unrestricted Filesystem Operations Through Restore Manifests
- Content
View full analysis
Path: if run_id == "latest": latest = backup_root / "latest" if latest.is_symlink(): return latest.resolve(strict=False) raise SystemExit("No latest backup run is available") run_dir = backup_root / run_id if not run_dir.is_dir(): raise SystemExit(f"Backup run not found: {run_id}") return run_dir def restore_run(run_dir: Path, apply: bool) -> dict: manifest_path = run_dir / "manifest.json" if not manifest_path.is_file(): raise SystemExit(f"Missing manifest: {manifest_path}") manifest = read_json(manifest_path) actions = manifest.get("actions", []) if not apply: return manifest for action in reversed(actions): destination = Path(action["path"]) if destination.is_symlink(): destination.unlink() elif destination.exists(): raise SystemExit( f"Restore blocked because destination is no longer a symlink: {destination}" ) backup_path = action.get("backup_path") if backup_path: source_backup = Path(backup_path) if not source_backup.exists() and not source_backup.is_symlink(): raise SystemExit(f"Missing backup payload: {source_backup}") destination.parent.mkdir(parents=True, exist_ok=True) shutil.move(str(source_backup), str(destination)) ``` ### Technical Analysis The value supplied through `--restore` is combined with `backup_root` without validating that it is a generated run identifier or that the resulting directory remains inside the configured backup directory. A value containing `..` components can escape `backup_root`. An absolute path ...[truncated 2917 chars]- Remediation
View remediation
/originals`. - Require every destination to match an approved Skill root recorded by the application. - Reject relative, malformed, duplicate, or unexpected destination paths. - Do not use unrestricted absolute paths directly from manifest data. 4. **Validate the complete manifest before applying any action** - Enforce a strict schema and supported manifest version. - Validate action types, required fields, path types, and expected source/destination relationships. - Complete all validation before performing the first filesystem mutation to avoid partial application. 5. **Add manifest integrity protection** - Store a cryptographic digest or authenticated signature when creating a restore manifest. - Verify integrity before applying restoration. - Ensure backup directories and manifests are writable only by the owning user. 6. **Use safer restore reconstruction** - Derive backup payload paths from validated destinations rather than accepting arbitrary `backup_path` values from JSON. - Record approved roots in trusted run metadata and reject destinations outside those roots. 7. **Add regression tests** - Test rejection of absolute restore identifiers. - Test rejection of `../` traversal. - Test a `latest` symlink resolving outside the backup root. - ...[truncated 193 chars]