T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/batch_convert.py:349
- Finding
- Unsafe Recursive Deletion of a Predictable Working Directory## Vulnerability Details **File Location**: `scripts/batch_convert.py`, lines 349-353 **Vulnerability Type**: Unsafe temporary-directory handling and recursive deletion **Risk Level**: Medium ```python temp_root = Path("./temp_batch").resolve() if temp_root.exists(): shutil.rmtree(temp_root) temp_root.mkdir(parents=True, exist_ok=True) ``` ### Technical Analysis The script uses the fixed path `./temp_batch` as its temporary directory. If that path already exists, the script recursively deletes it without verifying that it was created by this application, that it contains only disposable conversion data, or that it is safe to remove. The path is resolved relative to the process's current working directory rather than being created as a unique, process-owned temporary directory. Consequently, a legitimate directory named `temp_batch` may be mistaken for temporary application data. This deletion also occurs before the `--dry-run` early return at lines 377-385. Therefore, a mode described as performing only a scan can still delete and recreate a directory. The program does not use shell interpolation for this operation, so this is not command injection. The defect is unsafe filesystem lifecycle management involving an untrusted, predictable path. ### Attack Path 1. A user has a directory named `temp_batch` under the directory from which the Skill will be launched, and that directory contains unrelated files. 2. Alternatively, a local attacker who can write to the working directory creates or replaces `temp_batch` before execution. 3. The user or an Agent invokes `batch_convert.py`, including potentially with `--dry-run`. 4. The script resolves `./temp_batch` and sees that it exists. 5. `shutil.rmtree(temp_root)` recursively removes its contents without ownership or provenance validation. 6. The script recreates an empty directory at the same path, concealing the fact that the former contents were unrelated appli ...[truncated 620 chars]
- Remediation
- ## Remediation Suggestions 1. Replace the fixed working-directory path with a unique, process-owned temporary directory: ```python import tempfile with tempfile.TemporaryDirectory(prefix="document-organizer-") as temp_dir: temp_root = Path(temp_dir) # Perform conversion work within this context. ``` 2. Do not create, delete, or otherwise modify temporary paths before handling the `--dry-run` early return. 3. If a persistent temporary location is required, create a unique child directory using `tempfile.mkdtemp()` rather than deleting the parent directory. 4. Before cleanup, verify that the target: - Is beneath an explicitly designated temporary root. - Was created by the current process. - Is not the temporary root itself, the source directory, the output directory, or the current working directory. - Has not been replaced with an unexpected filesystem object. 5. Avoid recommending privileged execution as a general response to permission errors. Require only read access to the source and write access to dedicated output and temporary directories. 6. Add regression tests confirming that: - An existing `./temp_batch` directory is not deleted. - `--dry-run` performs no filesystem modifications. - Cleanup cannot remove the source or output directory.