T09 · Insecure Skill Coding Practices
Warning
- Location
- prompt_git.py:127
- Finding
- Unvalidated version identifiers permit path traversal during content reads<![CDATA[ ## Vulnerability Details **File Location**: `prompt_git.py:127-142` and `prompt_git.py:263-270` **Vulnerability Type**: Path traversal through repository metadata **Risk Level**: Medium ### Vulnerable Code ```python def _get_content_path(self, name: str, version_id: str) -> str: """Get content file path for a version.""" return os.path.join(self._get_prompt_dir(name), f'{version_id}.txt') def _load_versions(self, name: str) -> List[PromptVersion]: """Load version history for a prompt.""" versions_path = self._get_versions_path(name) if not os.path.exists(versions_path): return [] try: with open(versions_path, 'r', encoding='utf-8') as f: data = json.load(f) return [PromptVersion(**v) for v in data] ``` The resulting path is subsequently opened without verifying that it remains inside the prompt directory: ```python for v in versions: if v.id == version_id: # Load content content_path = self._get_content_path(name, v.id) try: with open(content_path, 'r', encoding='utf-8') as f: v.content = f.read() return v except FileNotFoundError: return None ``` ### Technical Analysis Prompt names are validated by `_sanitize_name()`, but version identifiers loaded from `versions.json` are trusted without equivalent validation. `_get_content_path()` directly appends the metadata-controlled identifier to a filesystem path. A version identifier such as `../../target` produces a path ending in `../../target.txt`. The operating system normalizes these traversal components when the file is opened, allowing the read operation to escape the intended prompt directory. The generated version identifiers are normally 16-character hexadecimal SHA-256 prefixes, so accepting path separators or identifiers outside that format is unnecessary. This issue is especially relevant to repositories obtained through the documented G ...[truncated 1412 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate every version identifier before using it as a path component. Since identifiers are generated as 16-character hexadecimal hashes, enforce that exact format: ```python import re def _validate_version_id(self, version_id: str) -> str: if not isinstance(version_id, str) or not re.fullmatch(r'[0-9a-f]{16}', version_id): raise ValueError("Invalid version identifier") return version_id ``` 2. Perform containment validation on the complete resolved file path: ```python def _get_content_path(self, name: str, version_id: str) -> str: self._validate_version_id(version_id) prompt_dir = os.path.realpath(self._get_prompt_dir(name)) candidate = os.path.realpath( os.path.join(prompt_dir, f'{version_id}.txt') ) if os.path.commonpath([prompt_dir, candidate]) != prompt_dir: raise ValueError("Content path escapes the prompt directory") return candidate ``` 3. Validate all fields loaded from `versions.json` and `index.json` against an explicit schema before constructing `PromptVersion` objects. 4. Reject inconsistent metadata where a current version is absent from the validated version history. 5. Add regression tests covering `../`, `..\`, absolute paths, null bytes, malformed hashes, and poisoned `index.json` or `versions.json` files. ]]>