T09 · Insecure Skill Coding Practices
- Location
src/mal_updater/crunchyroll_auth.py:72- Finding
Unsanitized Provider Profile Allows Token and State Path Traversal
- Content
View full analysis
CrunchyrollStatePaths: root = config.state_dir / "crunchyroll" / profile return CrunchyrollStatePaths( root=root, refresh_token_path=root / "refresh_token.txt", device_id_path=root / "device_id.txt", session_state_path=root / "session.json", sync_boundary_path=root / "sync_boundary.json", ) ``` ```python def resolve_hidive_state_paths(config: AppConfig, profile: str = "default") -> HidiveStatePaths: root = config.state_dir / "hidive" / profile return HidiveStatePaths( root=root, access_token_path=root / "authorisation_token.txt", refresh_token_path=root / "refresh_token.txt", session_state_path=root / "session.json", sync_boundary_path=root / "sync_boundary.json", ) ``` The `profile` value is exposed through provider CLI `--profile` arguments and is used directly as a filesystem path component. ### Technical Analysis Neither provider validates that `profile` is a simple profile identifier. A value containing `..` can escape the intended provider state directory. An absolute path is more severe because `pathlib` discards the preceding path components when joining it, making the absolute profile path the effective root. Authentication and snapshot operations subsequently create this directory and write fixed-name files beneath it. These files include provider refresh tokens, HIDIVE access tokens, device identifiers, session metadata, and synchronization boundaries. Although secret-writing functions apply mode `0600`, that protection does not restore containment. A token can still be placed in an unintended directory ...[truncated 1527 chars]- Remediation
View remediation
str: if profile in {".", ".."} or not PROFILE_RE.fullmatch(profile): raise ValueError("Invalid provider profile name") return profile ``` 2. Explicitly reject: - Absolute paths. - `/` and `\` path separators. - `.` and `..`. - Empty names. - Control characters and excessively long values. 3. Enforce path containment after resolution: ```python provider_root = (config.state_dir / "crunchyroll").resolve() root = (provider_root / validate_profile(profile)).resolve() if not root.is_relative_to(provider_root): raise ValueError("Provider profile escapes the state directory") ``` 4. Apply the same centralized validation to both Crunchyroll and HIDIVE instead of implementing provider-specific checks. 5. Add regression tests covering: - `../../escape` - `..` - Absolute paths - Embedded separators - Valid identifiers such as `default`, `family-1`, and `profile_2` 6. Where feasible, use directory file descriptors or equivalent safe-open patterns to reduce symlink-based path redirection risks. ]]>
