T09 · Insecure Skill Coding Practices
Error
- Location
- music_tagger.py:160
- Finding
- Tamperable Backup File Enables Arbitrary File Copy and Deletion<![CDATA[ ## Vulnerability Details **File Location**: `music_tagger.py:25-28`, `music_tagger.py:160-179` **Vulnerability Type**: Untrusted filesystem paths from mutable backup data **Risk Level**: High ### Vulnerable Code ```python def load_backup(self): if self.backup_file.exists(): with open(self.backup_file, 'r', encoding='utf-8') as f: self.backup_data = json.load(f) return self.backup_data ``` ```python def undo(self): backup = self.load_backup() if not backup: print("没有找到备份文件，无法撤销") return False reverse_mappings = {v: k for k, v in backup.items()} count = 0 for new_str, old_str in reverse_mappings.items(): new_path = Path(new_str) old_path = Path(old_str) if new_path.exists() and not old_path.exists(): copy2(new_path, old_path) new_path.unlink() print(f"撤销: {new_path.name} -> {old_path.name}") count += 1 if count > 0: self.backup_file.unlink(missing_ok=True) print(f"已撤销 {count} 个音乐文件的操作") return True else: print("没有需要撤销的音乐文件") return False ``` ### Technical Analysis The undo operation treats every key and value loaded from `.music-tagger-backup.json` as a trusted filesystem path. It does not validate the JSON schema, authenticate the backup, verify file ownership, reject symbolic links, or ensure that canonicalized paths remain within the intended input and output directories. After reversing the attacker-controlled mappings, the code uses one supplied path as the source of `copy2()` and another as its destination. It then unconditionally removes the source with `unlink()` after a successful copy. Consequently, a crafted backup can direct the application to relocate an arbitrary file that the invoking user can read and delete to any nonexistent path whose parent directory the user can write. This does not grant privileges beyond those of the user running the progra ...[truncated 1641 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Define and enforce a strict JSON schema requiring a dictionary of string paths with no unexpected fields or value types. 2. Resolve every path with `Path.resolve()` before use. 3. Require organized-file sources to remain beneath the configured output directory and restoration destinations to remain beneath the original input directory. 4. Reject absolute paths when relative paths are sufficient, as well as paths containing traversal components. 5. Reject symbolic links and verify file types with `lstat()` immediately before copying or deleting. 6. Store only normalized relative paths in the backup rather than unrestricted absolute paths. 7. Protect the backup with restrictive permissions and an integrity mechanism, such as an authenticated digest stored in a trusted location. 8. Before deleting a source, verify that it matches an operation created by the current application and that the copy completed successfully. 9. Present every resolved source and destination for confirmation when restoring files. 10. Avoid using mutable metadata as the sole authorization for destructive filesystem operations. ]]>