T09 · Insecure Skill Coding Practices
Error
- Location
- Mouse_produce_scratch.py:624
- Finding
- Unconditional Recursive Deletion of the Dataset Images Directory<![CDATA[ ## Vulnerability Details **File Location**: `Mouse_produce_scratch.py`, lines 624–633 **Vulnerability Type**: Destructive filesystem operation without validation or confirmation **Risk Level**: High ### Vulnerable Code ```python BASE_FOLDER = args.base_path DATA_FOLDER = BASE_FOLDER SAVE_FOLDER = BASE_FOLDER + '/images' MERGE_LABEL_FOLDER = BASE_FOLDER + '/labels' OUT_FOLDER = BASE_FOLDER + '/test' # SCRATCH_ID = args.scratch_id SCRATCH_ID = 3 LABEL_FOLDER = args.bbox_path os.makedirs(MERGE_LABEL_FOLDER, exist_ok=True) try: shutil.rmtree(SAVE_FOLDER) except: pass os.makedirs(SAVE_FOLDER, exist_ok=True) ``` ### Technical Analysis The `--base_path` command-line argument directly determines `SAVE_FOLDER`. On every invocation, the program recursively deletes the existing `<base_path>/images` directory using `shutil.rmtree()` before recreating it. The operation has no path validation, ownership check, backup, confirmation prompt, dry-run mode, or explicit overwrite option. It also uses a blanket `except` clause, which suppresses all deletion errors and prevents the user from understanding whether the filesystem operation completed safely. The deleted directory follows a conventional YOLO dataset layout and is therefore likely to contain original or valuable training images rather than disposable temporary files. This destructive behavior is not disclosed in `SKILL.md`. ### Attack Path 1. A user or automated agent is instructed to run scratch generation with a supplied `--base_path`. 2. The supplied path points to a legitimate dataset whose `images` subdirectory contains existing data. 3. `make_scratch()` constructs `SAVE_FOLDER` as `<base_path>/images`. 4. `shutil.rmtree(SAVE_FOLDER)` recursively removes that directory and its contents. 5. The program recreates an empty directory at the same location and begins writing generated output. 6. Unless an independent backup exists, the original images are irreversibly lost. An attacker does ...[truncated 891 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Never automatically delete a conventional input directory such as `<base_path>/images`. 2. Require a distinct output path, for example through a mandatory `--output_path` argument. 3. Refuse to use an existing non-empty output directory by default. 4. If replacement is necessary, require an explicit `--overwrite` flag and display the fully resolved path before deletion. 5. Resolve and validate paths with `pathlib.Path.resolve()` and reject unsafe targets such as filesystem roots, the base dataset directory itself, or paths outside an approved workspace. 6. Create a backup or use atomic directory replacement when existing output must be superseded. 7. Replace the blanket exception with specific exception handling and terminate safely when deletion fails. A safer pattern is: ```python from pathlib import Path base_folder = Path(args.base_path).resolve() output_folder = Path(args.output_path).resolve() if output_folder == base_folder or base_folder not in output_folder.parents: raise ValueError("Output directory must be a dedicated child of the dataset directory") if output_folder.exists() and any(output_folder.iterdir()): if not args.overwrite: raise FileExistsError( f"Output directory is not empty: {output_folder}. " "Use --overwrite to replace generated output." ) shutil.rmtree(output_folder) output_folder.mkdir(parents=True, exist_ok=False) ``` The destructive behavior and overwrite semantics must also be documented in `SKILL.md`. ]]>