T09 · Insecure Skill Coding Practices
- Location
scripts/batch_convert.py:349- Finding
Unsafe Recursive Deletion of a Predictable Working Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/batch_convert.py, lines 349-353
Vulnerability Type: Unsafe temporary-directory handling and recursive deletion
Risk Level: Mediumpython 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_batchas 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_batchmay be mistaken for temporary application data.This deletion also occurs before the
--dry-runearly 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
- A user has a directory named
temp_batchunder the directory from which the Skill will be launched, and that directory contains unrelated files. - Alternatively, a local attacker who can write to the working directory creates or replaces
temp_batchbefore execution. - The user or an Agent invokes
batch_convert.py, including potentially with--dry-run. - The script resolves
./temp_batchand sees that it exists. shutil.rmtree(temp_root)recursively removes its contents without ownership or provenance validation.- The script recreates an empty directory at the same path, concealing the fact that the former contents were unrelated appli ...[truncated 620 chars]
- A user has a directory named
- Remediation
View remediation
Remediation Suggestions
-
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. -
Do not create, delete, or otherwise modify temporary paths before handling the
--dry-runearly return. -
If a persistent temporary location is required, create a unique child directory using
tempfile.mkdtemp()rather than deleting the parent directory. -
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.
-
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.
-
Add regression tests confirming that:
- An existing
./temp_batchdirectory is not deleted. --dry-runperforms no filesystem modifications.- Cleanup cannot remove the source or output directory.
- An existing
-