T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/lib/storage.py:23
- Finding
- Predictable Temporary File Permits Symlink-Based File Overwrite## Vulnerability Details **File Location**: `scripts/lib/storage.py:23-27` **Vulnerability Type**: Unsafe temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```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 `_atomic_save` function uses a fixed temporary-file name derived by appending `.tmp` to the destination path. It opens that path with the normal `"w"` mode, which neither creates the file exclusively nor prevents symbolic-link traversal. If another local process can write to `~/.openclaw/workspace/memory/chart/`, it can create `charts.json.tmp` as a symbolic link to a file writable by the account running the skill. When the skill saves chart metadata, `open()` follows the symbolic link and truncates the target before writing JSON data. The subsequent `os.replace()` operation does not undo the modification already made through the link. ### Attack Path 1. An attacker obtains write access to the chart storage directory under the victim account. 2. The attacker creates `charts.json.tmp` as a symbolic link to another file that the victim account can write. 3. The attacker waits for or induces execution of `make_chart.py`, which eventually calls `save_charts()`. 4. `_atomic_save()` opens the predictable temporary path in write mode. 5. The operating system follows the symbolic link, truncating the linked target and replacing its contents with chart metadata. 6. The target file is corrupted or overwritten under the privileges of the account running the skill. ### Impact Assessment Successful exploitation permits corruption or overwrite of an arbitrary file writable by the skill's operating-system account. This could cause loss of user data or alter user-level configuration, depending on the selected ta ...[truncated 261 chars]
- Remediation
- ## Remediation Suggestions - Create the temporary file securely and unpredictably in the destination directory with `tempfile.NamedTemporaryFile(delete=False, dir=destination_directory, mode="w", encoding="utf-8")`. - Ensure the temporary file is created exclusively and is not a symbolic link. - Flush buffered data and call `os.fsync()` before atomically replacing the destination when durability is required. - Place `os.replace()` in a guarded workflow and remove the temporary file on failure. - Enforce owner-only permissions on the storage directory and metadata files, such as `0700` for directories and `0600` for files. - Where supported, use secure descriptor-based operations or no-follow semantics to further reduce symbolic-link race risks. A hardened implementation should resemble: ```python import os import tempfile def _atomic_save(path, data): ensure_dir() directory = os.path.dirname(path) tmp_path = None try: with tempfile.NamedTemporaryFile( mode="w", encoding="utf-8", dir=directory, prefix=".charts-", suffix=".tmp", delete=False, ) as f: tmp_path = f.name json.dump(data, f, indent=2, ensure_ascii=False) f.flush() os.fsync(f.fileno()) os.replace(tmp_path, path) tmp_path = None finally: if tmp_path is not None: try: os.unlink(tmp_path) except FileNotFoundError: pass ```