T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/get_nav.py:19
- Finding
- Predictable Shared Cache File Allows Symlink-Based File Overwrite and Cache Poisoning<![CDATA[ ## Vulnerability Details **File Location**: `scripts/get_nav.py`, lines 19–36 **Vulnerability Type**: Unsafe temporary file handling **Risk Level**: Medium ### Vulnerable Code ```python CACHE_PATH = Path("/tmp/mfapi-schemes.json") CACHE_MAX_AGE = 86400 # 24 hours in seconds def load_cache() -> List[Dict[str, Any]]: """Load scheme list from cache, refreshing if stale or missing.""" if CACHE_PATH.exists() and (time.time() - CACHE_PATH.stat().st_mtime) < CACHE_MAX_AGE: with open(CACHE_PATH) as f: return json.load(f) return refresh_cache() def refresh_cache() -> List[Dict[str, Any]]: """Download full scheme list and write to cache.""" data = api_get("/mf") CACHE_PATH.write_text(json.dumps(data)) return data ``` ### Technical Analysis The script stores cached API data at the fixed path `/tmp/mfapi-schemes.json`. On multi-user systems, `/tmp` is normally writable by every local user. Although the directory commonly has the sticky bit enabled, that does not prevent an attacker from creating a previously nonexistent file or symbolic link at this predictable path. The script does not: - Verify that the cache is a regular file rather than a symbolic link. - Verify that the cache is owned by the current user. - Apply restrictive file permissions explicitly. - Create the cache through an exclusive, race-resistant operation. - Write to a securely created temporary file and atomically replace the cache. - Protect the check-and-use sequence from time-of-check/time-of-use races. `Path.write_text()` follows an existing symbolic link. Therefore, during a cache refresh, a malicious link can redirect the write to another file writable by the victim account. The cache-reading path also accepts any sufficiently recent JSON file at the shared location, allowing another local user to supply forged scheme records. ### Attack Path #### Symlink-based file overwrite 1. A local attacker predicts the fixed cache path ` ...[truncated 1923 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Store the cache in a private, per-user cache directory, such as `$XDG_CACHE_HOME/mfapi` or `~/.cache/mfapi`, rather than directly under shared `/tmp`. 2. Create the cache directory with mode `0700` and verify that it is owned by the current user. 3. Before reading an existing cache, use `lstat()` to reject symbolic links and verify that the object is a regular file owned by the current user. 4. Write updates to a securely created temporary file in the same private directory, set its mode to `0600`, flush and synchronize it as appropriate, and atomically install it with `os.replace()`. 5. Avoid separate existence, metadata, and open operations where possible, because those operations introduce time-of-check/time-of-use race conditions. 6. Validate the decoded cache structure before trusting it. Confirm that the top-level value is a list and that each accepted record contains fields of the expected types. 7. Handle invalid, unreadable, or unexpectedly owned cache files by rejecting and securely recreating them rather than trusting their contents. A hardened design should resemble the following: ```python import os import stat import tempfile from pathlib import Path CACHE_DIR = Path(os.environ.get("XDG_CACHE_HOME", Path.home() / ".cache")) / "mfapi" CACHE_PATH = CACHE_DIR / "schemes.json" def prepare_cache_dir(): CACHE_DIR.mkdir(mode=0o700, parents=True, exist_ok=True) info = CACHE_DIR.stat() if info.st_uid != os.getuid() or not stat.S_ISDIR(info.st_mode): raise RuntimeError("Unsafe cache directory") def write_cache_atomically(data): prepare_cache_dir() fd, temporary_name = tempfile.mkstemp(dir=CACHE_DIR, prefix=".schemes-", text=True) try: os.fchmod(fd, 0o600) with os.fdopen(fd, "w") as output: json.dump(data, output) output.flush() os.fsync(output.fileno()) os.replace(temporary_name, CACHE_PATH) except Exception: try: ...[truncated 244 chars]