T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/credit_system.py:14
- Finding
- User-Controlled Path Traversal Enables Arbitrary JSON File Access and Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/credit_system.py`, lines 14–18 and 31–33 **Vulnerability Type**: Path traversal and unsafe file storage **Risk Level**: High ### Vulnerable Code ```python def __init__(self, user_id: str): self.user_id = user_id self.credit_file = Path(f"~/.openclaw/zh_semantic_credits/{user_id}.json").expanduser() self.credit_file.parent.mkdir(parents=True, exist_ok=True) self.data = self._load() def _save(self): with open(self.credit_file, 'w', encoding='utf-8') as f: json.dump(self.data, f, ensure_ascii=False, indent=2) ``` ### Technical Analysis The `user_id` value is interpolated directly into a filesystem path without validation, normalization, or a canonical containment check. A value containing traversal components such as `../` can cause `self.credit_file` to resolve outside the intended `~/.openclaw/zh_semantic_credits` directory. The constructor also creates the resolved parent directories. Subsequent calls to `_load()` and `_save()` can therefore read from or overwrite a caller-selected path ending in `.json`, subject to the operating-system permissions of the Skill process. There is no protection against symbolic links. If an attacker can place a symlink at the calculated location, writes may also be redirected to another file. ### Attack Path 1. An attacker reaches code that constructs `CreditSystem` and supplies a crafted `user_id`. 2. The value contains sufficient `../` components to escape the intended credit directory. 3. `Path.expanduser()` expands the home directory but does not remove or reject traversal. 4. `mkdir(parents=True)` creates attacker-selected parent directories when possible. 5. The attacker triggers `use_credit()` or `add_credits()`, which invokes `_save()`. 6. The resolved `.json` file is created or overwritten with the credit-state document. Reading an existing target through `_load()` requires it to contain valid JSON. Creating or overwriting ...[truncated 516 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate `user_id` against a strict allowlist, such as `^[A-Za-z0-9_-]{1,64}$`. 2. Prefer deriving the filename from a cryptographic hash of the identifier rather than using the raw identifier. 3. Resolve the storage root and target path before use and verify that the target remains inside the storage root. 4. Reject absolute paths, traversal components, path separators, null bytes, and unexpected Unicode separator characters. 5. Refuse symbolic-link targets and use secure file-opening flags where supported. 6. Create the storage directory with mode `0700` and state files with mode `0600`. 7. Use atomic writes through a securely created temporary file followed by an in-directory rename. ]]>