T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/disk_butler.py:89
- Finding
- Environment-Controlled Cleanup Roots Can Cause Deletion Outside Trusted Cache Locations<![CDATA[ ## Vulnerability Details **File Location**: `scripts/disk_butler.py`, lines 89–96, 357–381, and 450–474 **Vulnerability Type**: Untrusted path handling in a destructive file operation **Risk Level**: High ### Vulnerable Code ```python USERPROFILE = os.environ.get("USERPROFILE", os.path.expanduser("~")) LOCALAPPDATA = os.environ.get("LOCALAPPDATA", os.path.join(USERPROFILE, "AppData", "Local")) APPDATA = os.environ.get("APPDATA", os.path.join(USERPROFILE, "AppData", "Roaming")) SYSTEMROOT = os.environ.get("SystemRoot", r"C:\Windows") ``` ```python def temp_roots(): roots = set() for v in [os.environ.get("TEMP"), os.environ.get("TMP"), os.path.join(SYSTEMROOT, "Temp"), os.path.join(LOCALAPPDATA, "Temp")]: if v and os.path.isdir(v) and not is_r3(v): roots.add(v) return roots def find_temp_targets(min_age_days): out = [] cutoff = time.time() - min_age_days * 86400 for root in temp_roots(): try: names = os.listdir(root) except OSError: continue for name in names: p = os.path.join(root, name) if is_r3(p): continue try: st = os.stat(p, follow_symlinks=False) except OSError: continue if st.st_mtime > cutoff: continue # Too new, skip if os.path.islink(p) or _is_junction_path(p): continue sz = walk_size(p) if os.path.isdir(p) else st.st_size out.append({"label": "System Temp", "path": p, "size": sz, "risk": "R1", "reason": f"Temporary file created over {min_age_days} days ago"}) return out ``` ```python def safe_remove(path, dry, fh): if is_r3(path): add_lesson(f"Blocked deletion: {path} matched an R3 zone", tag="automatic") return False ...[truncated 3458 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not treat environment variables as authoritative cleanup roots. Obtain standard Windows directories through trusted operating-system APIs, such as the appropriate Known Folder APIs. 2. Canonicalize every root and deletion target with `os.path.realpath()` and normalized case handling. 3. Maintain an immutable allowlist of canonical cleanup roots and require component-aware containment: ```python def is_within(path, root): path = os.path.realpath(path) root = os.path.realpath(root) return os.path.commonpath([path, root]) == root ``` 4. Immediately before deletion, require the target to be strictly beneath—not equal to—one approved root. 5. Reject relative paths, drive-relative paths, UNC paths, unexpected volumes, and roots whose ownership or access-control properties do not match expectations. 6. Revalidate links, junctions, and canonical containment immediately before the destructive operation to reduce path-substitution risks. 7. Display both the configured and canonical paths in the dry-run plan. 8. Add tests that substitute `TEMP`, `TMP`, `LOCALAPPDATA`, `APPDATA`, `USERPROFILE`, and `SystemRoot` with arbitrary directories and verify that cleanup is refused. ]]>