T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/utils.py:16
- Finding
- Predictable Shared Cache Permits Symlink Following and Path Traversal## Vulnerability Details **File Location**: `scripts/utils.py`, lines 16-22, 44-55, and 122 **Vulnerability Type**: Unsafe temporary-file and path construction **Risk Level**: High ### Vulnerable Code ```python # Cache directory CACHE_DIR = Path("/tmp/stock_analysis_cache") CACHE_DIR.mkdir(exist_ok=True) def cache_path(ticker: str, data_type: str, suffix: str = "json") -> Path: """Generate cache file path.""" return CACHE_DIR / f"{ticker}_{data_type}.{suffix}" ``` ```python cache_file = cache_path(ticker, f"kline_{days}") if is_cache_valid(cache_file): try: with open(cache_file, 'r') as f: return json.load(f) except: pass # Cache read failed, fetch fresh url = f"https://web.ifzq.gtimg.cn/appstock/app/fqkline/get?_var=kline_dayqfq&param={ticker},day,,,{days},qfq" try: resp = requests.get(url, timeout=10) ``` ```python with open(cache_file, 'w') as f: json.dump(result, f, ensure_ascii=False, indent=2) ``` ### Technical Analysis The cache uses a fixed, predictable directory under the globally shared `/tmp` hierarchy. The directory is created with `exist_ok=True`, but the code does not verify: - Whether an existing path is a real directory rather than a symlink. - Whether the directory is owned by the current process user. - Whether its permissions prohibit modification by other local users. - Whether an individual cache entry is a regular file rather than a symbolic link. The ticker is also directly incorporated into the cache path without format validation. Path separators and `..` components are therefore not rejected, allowing the resolved cache path to escape the intended directory in execution contexts where an attacker controls the ticker. Ordinary `open(..., 'w')` follows symbolic links and truncates the target. Consequently, a maliciously prepared cache entry can redirect a successful cache write to another file writable by the Skill process. Cache reads also follow symbolic lin ...[truncated 1932 chars]
- Remediation
- ## Remediation Suggestions 1. Strictly validate ticker values before they reach either the URL or filesystem: ```python TICKER_PATTERN = re.compile(r"^(sh|sz)\d{6}$") def validate_ticker(ticker: str) -> str: if not TICKER_PATTERN.fullmatch(ticker): raise ValueError("Invalid ticker format") return ticker ``` 2. Validate `days` as an integer within the supported range, such as 1 through 320. 3. Use a per-user private cache directory rather than a predictable shared directory. Create it with mode `0700` and verify ownership. 4. Resolve every generated path and verify that it remains under the resolved cache root: ```python candidate = (CACHE_DIR / filename).resolve() candidate.relative_to(CACHE_DIR.resolve()) ``` 5. Reject cache paths that are symbolic links or non-regular files. 6. Open files using operating-system flags that prevent symlink following, such as `O_NOFOLLOW` where supported. 7. Write to a securely created temporary file in the same directory, flush and synchronize it, and then atomically replace the destination. 8. Do not silently accept an attacker-created cache directory. Verify that the directory is owned by the expected user and is not group- or world-writable.