T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/quality_filter.py:204
- Finding
- Destination File Overwrite During Move Operations<![CDATA[ ## Vulnerability Details **File Location**: `scripts/quality_filter.py:204-207` **Vulnerability Type**: Unsafe file move with destination filename collisions **Risk Level**: Medium ### Vulnerable Code ```python basename = os.path.basename(r['path']) dest = os.path.join(output_dir, basename) shutil.move(r['path'], dest) moved += 1 ``` ### Technical Analysis The recursive scanner can discover files with identical basenames in different source subdirectories. During a move operation, the code discards each source file's relative directory structure and places every low-quality image directly into one output directory. The destination path is not checked for an existing file and is not assigned a collision-resistant name. On platforms where the underlying move or rename operation replaces an existing destination, a later image can silently overwrite an earlier image or a file that existed in the output directory before the operation. The move action does not require confirmation, unlike the delete action. This increases the likelihood that data loss will occur without the user being given an opportunity to review the operation. ### Attack Path 1. Create two subdirectories under the scanned directory. 2. Place a low-quality image with the same basename, such as `photo.jpg`, in each subdirectory. 3. Alternatively, place an existing `photo.jpg` in the selected output directory. 4. Run the tool with `scan <directory> --action move --output <output-directory>`. 5. Both source files resolve to `<output-directory>/photo.jpg`. 6. Depending on platform filesystem semantics, a subsequent `shutil.move` replaces the existing destination. 7. The previously moved or pre-existing destination file is lost. ### Impact Assessment The issue can cause unintended and potentially irreversible loss of image files. Its scope is limited to files writable by the account executing the script and to destination names derived from identified low-quality images. It does not ...[truncated 113 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Preserve each source file's path relative to the scanned root instead of flattening all files into one directory. - Before moving a file, explicitly test whether the destination already exists and refuse to overwrite it. - Generate a unique destination name when a collision occurs, such as by adding a counter or a cryptographically strong content-derived suffix. - Prefer an operation that enforces exclusive destination creation rather than relying on platform-specific `shutil.move` overwrite behavior. - Add a confirmation or dry-run summary for move operations. - Record collision decisions and failures in the output so users can verify that every source file was handled safely. Example defensive logic: ```python basename = os.path.basename(r['path']) dest = os.path.join(output_dir, basename) if os.path.exists(dest): stem, suffix = os.path.splitext(basename) counter = 1 while os.path.exists(dest): dest = os.path.join(output_dir, f"{stem}_{counter}{suffix}") counter += 1 shutil.move(r['path'], dest) moved += 1 ``` ]]>