T09 · Insecure Skill Coding Practices
- Location
src/synthclaw/blender/cleanup.py:3- Finding
Recursive Deletion of Unrelated TIFF Files in User-Controlled Output Directory
- Content
View full analysis
Vulnerability Details
File Location:
src/synthclaw/blender/cleanup.py:3-12
Call Site:scripts/render_dataset.py:57-59
Vulnerability Type: Unrestricted destructive file cleanup
Risk Level: MediumVulnerable Code
python def clean_up_tifs(output_dir): """ Walks the output directories and removes unnecessary .tif files. """ for root, dirs, files in os.walk(output_dir): for file in files: if file.endswith('.tif'): try: os.remove(os.path.join(root, file)) print(f"[SynthClaw] CleanUp: Deleted temporary TIFF file {os.path.join(root, file)}") except Exception as e: print(f"[SynthClaw] CleanUp: Failed to delete TIFF file {file}: {e}")The function is invoked after dataset rendering:
python print("\n--- [SynthClaw] Render Loop Complete. Cleaning up files... ---") clean_up_tifs(output_dir) print("[SynthClaw] Complete!")Technical Analysis
render_procedural_datasetaccepts a caller-selectedoutput_dir, which may be an existing directory containing user data. After rendering,clean_up_tifsrecursively traverses that entire directory tree and deletes every file whose name ends with lowercase.tif.The cleanup routine does not establish that a file:
- Was generated by the current render operation.
- Is a temporary artifact.
- Resides in a dedicated per-run directory.
- Matches a tracked list of expected render outputs.
- Is safe to delete after resolving symbolic links and canonical paths.
Consequently, the cleanup operation has a broader deletion scope than required by the rendering task. It treats all matching files under the output tree as temporary, including unrelated pre-existing TIFF images.
Attack Path
- An attacker, untrusted agent instruction, or accidental user input selects an existing directory as ` ...[truncated 1461 chars]
- Remediation
View remediation
Remediation Suggestions
Replace extension-based recursive cleanup with explicit artifact tracking:
- Record the exact paths of temporary files created during the current render operation.
- Delete only paths contained in that trusted collection.
- Canonicalize each candidate with
os.path.realpathand verify confinement usingos.path.commonpath. - Reject candidates outside a dedicated per-run temporary directory.
- Create each render in a new directory using
tempfile.mkdtemp, then move only intended final outputs to the user-selected destination. - Avoid recursively cleaning a pre-existing user directory.
- If broad cleanup remains necessary, require explicit opt-in and return the candidate file list for confirmation before deletion.
- Report cleanup failures and deleted paths through the API result rather than only printing them.
A safer design is:
python def clean_up_generated_files(output_dir, generated_temp_files): output_root = os.path.realpath(output_dir) for candidate in generated_temp_files: candidate_path = os.path.realpath(candidate) if os.path.commonpath([output_root, candidate_path]) != output_root: raise ValueError(f"Cleanup path escapes output directory: {candidate}") if os.path.isfile(candidate_path): os.remove(candidate_path)The
generated_temp_filescollection must be populated when the renderer creates each temporary artifact, rather than reconstructed by recursively searching for a filename extension.