T09 · Insecure Skill Coding Practices
Warning
- Location
- download_organizer.py:31
- Finding
- Untrusted Backup Mappings Allow Arbitrary File Copy and Deletion## Vulnerability Details **File Location**: `download_organizer.py:31-34, 95-108` **Vulnerability Type**: Unvalidated file paths from a writable backup file **Risk Level**: Medium ### 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 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 ``` ### Technical Analysis The `undo` operation treats every source and destination path in `.download-organizer-backup.json` as trusted. The code does not validate the JSON schema, canonicalize and constrain paths to approved directories, reject symbolic links, or verify that each mapping was created by a previous legitimate organization operation. During undo, the JSON mapping is reversed. An attacker-controlled mapping can therefore select any existing path as `new_path` and any absent path as `old_path`. If the process has sufficient permissions, `copy2(new_path, old_path)` copies the selected source to the attacker-chosen destination, after which `new_path.unlink()` deletes the source. The `old_path.exists()` check limits straightforward replacement of an existing destination but does not prevent creation at an arbitrary absent path. It also introduces a time-of-check/time-of-use window because the destination is checked separately from the subsequent copy operation. ### Attack Path 1. The attacker gains write access to the output directory or plants/modifies `.download-organizer-backup.json`. 2. The attacker inserts a JSON mapping whose key is an arbitrary desired restoration destina ...[truncated 1144 chars]
- Remediation
- ## Remediation Suggestions 1. Store only normalized relative paths in the backup rather than unrestricted absolute paths. 2. Resolve every source and destination using `Path.resolve()` and require them to remain under explicitly configured input and output roots. 3. Reject mappings containing traversal components, unexpected value types, or paths outside the authorized roots. 4. Define and enforce a strict JSON schema before processing any mappings. 5. Reject symbolic links in the backup path, source paths, destination paths, and relevant parent directories. 6. Protect the backup file with restrictive permissions and refuse to load files owned by an unexpected user. 7. Revalidate containment and file type immediately before every copy and deletion. 8. Avoid check-then-act destination handling. Use filesystem operations that fail atomically if the destination already exists. 9. Delete a source only after confirming that it is an expected organized copy and that the restoration completed successfully. 10. Consider authenticating backup contents or storing operation state in a user-private application data directory.