T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/splitter.py:147
- Finding
- Destructive Default File Moves with Unchecked Destination Overwrites## Vulnerability Details **File Location**: `scripts/splitter.py:147-170` **Vulnerability Type**: Unsafe file operations and destructive default behavior **Risk Level**: Medium ```python def process_files(files, dest_dir, src_ann_dir=None, dest_ann_dir=None): count = 0 for img_path in files: img_name = Path(img_path).name if args.copy: dest_path = os.path.join(dest_dir, img_name) shutil.copy2(img_path, dest_path) else: dest_path = os.path.join(dest_dir, img_name) shutil.move(img_path, dest_path) # Process annotations if args.annotations and src_ann_dir and dest_ann_dir: ann_name = Path(img_path).stem + ".txt" src_ann = Path(src_ann_dir) / ann_name if src_ann.exists(): dest_ann = Path(dest_ann_dir) / ann_name if args.copy: shutil.copy2(src_ann, dest_ann) else: shutil.move(src_ann, dest_ann) count += 1 ``` ### Technical Analysis The split operation moves source images and associated annotations unless the user explicitly supplies the `--copy` option. The implementation performs no destination collision check, non-empty-directory check, overwrite confirmation, dry run, transactional staging, or rollback. Both `shutil.move()` and `shutil.copy2()` can replace an existing destination file when the destination resolves to an existing filename. Consequently, repeated executions, an incorrectly selected output directory, or pre-positioned files with matching names can cause existing output content to be silently replaced. Because files are processed sequentially, an exception during execution can leave the source and destination datasets in a partially modified state. ### Attack Path 1. A user invokes the documented split command without `--copy`, ...[truncated 1241 chars]
- Remediation
- ## Remediation Suggestions - Make copying the safe default and require an explicit `--move` option for destructive behavior. - Before processing, validate that the source exists, the destination is distinct from the source, and every destination path remains inside the intended output directory. - Reject non-empty output directories by default. Add an explicit `--overwrite` option if replacement is intentionally required. - Precompute all source and destination paths and detect filename collisions before performing any mutation. - Add a `--dry-run` mode that lists planned copies, moves, and conflicts. - Stage files in a temporary output directory and atomically rename the completed dataset into place only after all operations succeed. - If moving files is supported, maintain an operation journal and implement rollback for failures. - Validate split ratios before file operations, requiring non-negative values whose sum is exactly 100. - Clearly warn users before destructive operations and request confirmation in interactive use.