T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/zipcracker_core.py:2790
- Finding
- User-Controlled Output Path Enables Recursive Deletion of Arbitrary Directories## Vulnerability Details **File Location**: `scripts/zipcracker_core.py`, lines 2790-2796; user-controlled value assigned at lines 4320-4331 **Vulnerability Type**: Unrestricted recursive filesystem deletion **Risk Level**: High ### Vulnerable Code ```python def _clean_and_create_outdir(out_dir: str) -> None: if os.path.exists(out_dir): try: shutil.rmtree(out_dir) except Exception: pass os.makedirs(out_dir, exist_ok=True) ``` The deleted path comes directly from the command-line argument: ```python if arg in ("-o", "--out"): if index + 1 >= len(sys.argv): print( loc( locale, "[!] Error: No directory name provided after -o.", "[!] Error: No directory name provided after -o.", ) ) return 1 out_dir = sys.argv[index + 1] index += 2 ``` ### Technical Analysis The `-o` or `--out` option accepts an unrestricted path. Before extraction, `_clean_and_create_outdir()` recursively deletes that path with `shutil.rmtree()`. The code does not: - Canonicalize the destination before deletion. - Require the destination to be a dedicated ZipCracker directory. - Reject filesystem roots, home directories, the current working directory, or project directories. - Check for a tool-created ownership marker. - Request confirmation before deleting an existing directory. - Prevent paths containing symbolic-link-based redirections in parent components. This behavior is not required for ZIP recovery. Creating a new output directory or refusing to overwrite an existing directory would provide the declared functionality with substantially lower privileges. ### Attack Path 1. An attacker influences a natural-language request or generated command so that it includes a sensitive destination, such as `-o .`, `-o /home/user`, or another writable directory. ...[truncated 835 chars]
- Remediation
- ## Remediation Suggestions - Resolve the requested destination with `Path.resolve()` before performing any filesystem operation. - Reject filesystem roots, drive roots, user home directories, the current working directory, the archive's parent directory, and the Skill installation directory. - Default to a newly created, unique directory rather than deleting an existing one. - Refuse to overwrite a non-empty existing directory unless the user provides a separate explicit destructive flag. - Place a ZipCracker ownership marker inside directories created by the tool and only permit automatic cleanup when that marker is present and valid. - Check every parent component for symbolic links before recursive deletion. - Do not suppress deletion exceptions; report the failure and stop extraction. - Consider using `tempfile.mkdtemp()` followed by an atomic rename into a previously nonexistent destination.