T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/clean_cache.py:284
- Finding
- Symbolic Link Resolution Causes Deletion of the Link Target<![CDATA[ ## Vulnerability Details **File Location**: `scripts/clean_cache.py`, lines 284–306 and 546–563 **Vulnerability Type**: Unsafe symbolic-link handling leading to arbitrary directory deletion **Risk Level**: High ### Vulnerable Code ```python def assert_safe(path: str) -> str: """删除前最终闸门。 1. 用 os.path.realpath 解析符号链接 / junctions，得到真实路径； 2. 真实路径必须严格位于某个允许根区之下（边界安全）； 3. 命中系统关键片段（FORBIDDEN_PARTS）一律拒绝； 4. 拒绝驱动器根目录。 返回解析后的真实路径（供后续删除使用）。 """ try: real = os.path.realpath(os.path.abspath(path)) except Exception: real = os.path.abspath(path) real_norm = real.rstrip(os.sep).lower() if len(real_norm) == 2 and real_norm[1] == ":": raise RuntimeError(f"拒绝删除驱动器根目录: {real}") low = real.lower() for bad in FORBIDDEN_PARTS: if bad in low: raise RuntimeError(f"禁止删除系统关键路径: {real}") if not any(_is_under(real_norm, r) for r in _allowed_roots_norm()): raise RuntimeError(f"拒绝删除白名单外的路径: {real}") return real ``` The resolved path is subsequently stored in the deletion plan: ```python def collect(items, min_bytes): """计算大小并过滤，返回 (可选计划, 被拒绝项, 被占用跳过项)。""" plan, rejected, busy = [], [], [] for it in items: try: safe = assert_safe(it["path"]) except RuntimeError as e: rejected.append((it["name"], str(e))) continue if it.get("guard") and process_running(it["guard"]): busy.append(it["name"]) continue size = dir_size(safe) if size < min_bytes: continue it = dict(it) it["path"], it["size"] = safe, size plan.append(it) ``` ### Technical Analysis The deletion safety design claims that symbolic links and junctions are removed without recursively following their targets. However, `assert_safe()` resolves the selected path through `os.path.realpath()` and returns the resolved target rather than the original link path. `collect()` ...[truncated 1867 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not return the resolved target from `assert_safe()`. Preserve and return the original absolute path after validation. - Reject top-level symbolic links and junctions, or explicitly remove only the link itself. - Validate the original lexical path and its resolved parent separately: - Confirm the original path is under an approved cache root. - Resolve and validate the parent directory. - Use `lstat()` to identify the final component without following it. - On Windows, explicitly detect reparse points and junctions instead of relying only on `os.path.islink()`. - Narrow `ALLOWED_ROOTS`; do not treat the entire user home directory as an unrestricted deletion root. - Re-run safety validation immediately before deletion to reduce time-of-check/time-of-use race exposure. - Add automated tests for: - A cache path that is a symbolic link to a project directory. - Nested symbolic links. - Windows junctions and reparse points. - Links replaced between planning and deletion. ]]>