T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/state_store.py:46
- Finding
- Unrestricted State Key Allows Path Traversal and Arbitrary JSON File Access<![CDATA[ ## Vulnerability Details **File Location**: `scripts/state_store.py:46-73`; input originates from `scripts/run.py:34` and `scripts/run.py:49` **Vulnerability Type**: Path traversal and unsafe file access **Risk Level**: High ### Vulnerable Code ```python # scripts/run.py:34 prep.add_argument("--state-key") # scripts/run.py:49 commit.add_argument("--state-key") ``` ```python # scripts/state_store.py:46-73 def state_path(repo: RepoSpec, state_key: str | None = None) -> Path: key = state_key or repo.state_key return DEFAULT_STATE_ROOT / f"{key}.json" def load_state(repo: RepoSpec, state_key: str | None = None) -> tuple[StateData, Path, bool]: path = state_path(repo, state_key) path.parent.mkdir(parents=True, exist_ok=True) if not path.exists(): return StateData(repo=repo.slug), path, True data = json.loads(path.read_text(encoding="utf-8")) state = StateData( repo=data.get("repo", repo.slug), processed_tags=list(data.get("processed_tags", [])), latest_processed_release_id=data.get("latest_processed_release_id"), latest_processed_published_at=data.get("latest_processed_published_at"), last_checked_at=data.get("last_checked_at"), last_success_at=data.get("last_success_at"), initialized_at=data.get("initialized_at"), ) is_first_run = not bool(state.initialized_at) return state, path, is_first_run def save_state(path: Path, state: StateData) -> None: path.parent.mkdir(parents=True, exist_ok=True) path.write_text(json.dumps(state.to_dict(), ensure_ascii=False, indent=2) + "\n", encoding="utf-8") ``` ### Technical Analysis The `--state-key` command-line value is inserted directly into a filesystem path without validation, normalization, or a containment check. A state key containing parent-directory components such as `../` can cause the resulting path to escape `DEFAULT_STATE_ROOT`. Both cron preparation and commit operations call `load_ ...[truncated 2205 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict state keys to a conservative filename-safe format and length: ```python import re STATE_KEY_PATTERN = re.compile(r"^[A-Za-z0-9._-]{1,128}$") def validate_state_key(key: str) -> str: if not STATE_KEY_PATTERN.fullmatch(key): raise ValueError("invalid state key") if key in {".", ".."}: raise ValueError("invalid state key") return key ``` 2. Resolve the state root and candidate path, then enforce containment: ```python def state_path(repo: RepoSpec, state_key: str | None = None) -> Path: key = validate_state_key(state_key or repo.state_key) root = DEFAULT_STATE_ROOT.resolve() candidate = (root / f"{key}.json").resolve() if candidate.parent != root: raise ValueError("state path escapes the configured state root") return candidate ``` 3. Explicitly reject `/`, `\`, absolute paths, drive prefixes, null bytes, and parent-directory components for cross-platform safety. 4. Protect against symbolic-link attacks. Refuse state paths that are symlinks and, where supported, use no-follow file operations. 5. Write state atomically through a securely created temporary file in the same directory, flush and synchronize it as appropriate, and replace the destination only after serialization succeeds. 6. Consider setting restrictive state directory and file permissions because release state should not be writable by unrelated local users. 7. Add automated tests for: - Unix and Windows traversal sequences. - Absolute paths and drive-qualified paths. - Nested separators. - Symbolic-link targets. - Excessively long state keys. - Valid default and explicitly supplied state keys. - Verification that every resolved path remains inside the configured state root. ]]>