T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/dup_finder.py:3
- Finding
- Irreversible File Deletion Without Collision-Resistant Equality Verification or Backup<![CDATA[ ## Vulnerability Details **File Location**: `scripts/dup_finder.py:3-6, 31-50`; related inaccurate backup claim at `SKILL.md:54` **Vulnerability Type**: Unsafe destructive file operation and weak duplicate verification **Risk Level**: High ### Vulnerable Code ```python def sha_of(path, full): h = hashlib.sha1() with open(path, "rb") as f: h.update(f.read(65536) if not full else f.read()) return h.hexdigest() ``` ```python for ps in cand: by_head = {} for p in ps: by_head.setdefault(sha_of(p, False), []).append(p) for ps2 in by_head.values(): if len(ps2) > 1: by_full = {} for p in ps2: by_full.setdefault(sha_of(p, True), []).append(p) for ps3 in by_full.values(): if len(ps3) > 1: groups.append(sorted(ps3)) waste = sum(os.path.getsize(g[i]) for g in groups for i in range(1, len(g))) removed = 0 if a.apply: for g in groups: for p in g[1:]: try: os.remove(p); removed += 1 except OSError: pass ``` The documentation additionally states: ```markdown - 涉及写盘的操作默认 **dry-run 预览**，加 `--apply` 才执行（text-replace/dup-finder 还会先备份） ``` ### Technical Analysis The implementation considers two files identical solely because their sizes, first-block SHA-1 hashes, and complete SHA-1 hashes match. SHA-1 is collision-broken and is not suitable as the final proof that files are byte-for-byte identical. No final binary comparison is performed before deletion. There is also a time-of-check-to-time-of-use window between hashing and `os.remove()`. A file can be changed or replaced after it has been hashed but before deletion. The script neither revalidates the file identity and contents nor protects against path replacement immediately before removal. When `--apply` is supplied, duplicate candidates are permanently removed with `os.remove()`. No backup, quarantine, recycl ...[truncated 1453 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace SHA-1 with a collision-resistant digest such as SHA-256 or BLAKE2 for candidate grouping. 2. Treat hashes only as an optimization. Before deletion, compare every candidate against the retained file byte-for-byte using bounded chunks. 3. Immediately before removal, reopen and revalidate both files. Record and compare stable identity information where supported, including device, inode, size, modification time, and file type. 4. Avoid permanent deletion by default. Move candidates into a user-controlled quarantine directory or use an appropriate recoverable trash mechanism. 5. If backup behavior is promised, create and verify a backup before removing the original. Abort deletion if the backup cannot be completed. 6. Require explicit confirmation or a reviewed manifest for destructive operations, especially when processing broad directory trees. 7. Report every failed deletion and return a nonzero status for partial failure rather than silently suppressing `OSError`. 8. Correct `SKILL.md:54` until backup functionality is actually implemented and verified. ]]>