T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/lib/storage.py:24
- Finding
- Predictable Temporary File Enables Local Symlink-Based File Clobbering## Vulnerability Details **File Location**: `scripts/lib/storage.py`, lines 24–29 **Vulnerability Type**: Predictable and symlink-following temporary file **Risk Level**: Medium ```python def _atomic_save(path, data): ensure_dir() tmp = path + ".tmp" with open(tmp, "w", encoding="utf-8") as f: json.dump(data, f, indent=2, ensure_ascii=False) os.replace(tmp, path) ``` ### Technical Analysis The storage implementation derives a deterministic temporary filename by appending `.tmp` to the destination path. For example, writes to `items.json` always use `items.json.tmp`. Python's `open(tmp, "w")` follows symbolic links and truncates the resolved target. If an attacker who can manipulate entries in `~/.openclaw/workspace/memory/todo/` creates the predictable temporary path as a symbolic link, a subsequent save can overwrite another file writable by the Skill's operating-system user. The fixed temporary filename also creates a race between concurrent Skill processes. Two writers can open or replace the same temporary file, potentially causing failed operations, inconsistent state, or lost task data. ### Attack Path 1. A local attacker obtains the ability to create or replace directory entries in `~/.openclaw/workspace/memory/todo/`. 2. The attacker predicts a temporary path such as `items.json.tmp`, `stats.json.tmp`, or `archive.json.tmp`. 3. The attacker creates that path as a symbolic link to another file writable by the victim user. 4. The victim invokes an operation that saves task, statistics, or archive data. 5. `_atomic_save()` opens the predictable path in write mode. 6. The operating system follows the symbolic link and truncates the target before writing JSON content. 7. `os.replace()` subsequently moves the temporary directory entry to the intended JSON destination, but the target file has already been corrupted. ### Impact Assessment Exploitation requires local access suffic ...[truncated 515 chars]
- Remediation
- ## Remediation Suggestions - Create temporary files securely and uniquely in the destination directory with `tempfile.NamedTemporaryFile(delete=False, dir=TODO_DIR)` or `tempfile.mkstemp()`. - Write through the returned file descriptor rather than reopening a predictable pathname. - Flush buffered data and call `os.fsync()` before atomically replacing the destination. - Place cleanup in a `finally` block so temporary files are removed after failures. - Configure restrictive permissions for the storage directory and files, such as owner-only access where appropriate. - Reject or safely handle symbolic links in the storage path. - Add inter-process file locking or another concurrency-control mechanism if simultaneous writers are supported. - Consider validating that the destination directory is owned by the expected user and is not writable by untrusted users.