T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/easyclaw_unzip_skill.py:121
- Finding
- ZIP Path Traversal Allows Arbitrary File Overwrite## Vulnerability Details **File Location**: `scripts/easyclaw_unzip_skill.py`, lines 121–140 **Vulnerability Type**: ZIP path traversal **Risk Level**: High **Vulnerable Code:** ```python if member.startswith(root_folder + '/'): # Calculate relative path relative_path = member[len(root_folder) + 1:] if not relative_path: # Skip empty paths continue target_file = os.path.join(target_path, relative_path) # If it's a folder if member.endswith('/'): os.makedirs(target_file, exist_ok=True) else: # Ensure parent directory exists parent_dir = os.path.dirname(target_file) if parent_dir: os.makedirs(parent_dir, exist_ok=True) # Extract file with zip_file.open(member) as source, open(target_file, 'wb') as target: target.write(source.read()) ``` ### Technical Analysis The single-root-folder extraction branch removes the archive's root prefix and directly combines the remaining member name with `target_path`. It does not normalize the resulting path or verify that the resolved destination remains inside the intended extraction directory. An archive member such as `skill/../../outside.txt` satisfies the `member.startswith(root_folder + '/')` check. After removing the root prefix, `relative_path` becomes `../../outside.txt`. Passing that value to `os.path.join()` does not remove the traversal components. The subsequent `os.makedirs()` and `open(..., 'wb')` operations therefore act on a location outside `target_path`. The filename validation at lines 93–98 applies only to the ZIP archive's base filename, not to individual archive members, so it does not prevent this attack. The risk is amplified because `SKILL.md` instructs the Agent to use this script when processing user-provided Skill archives. ### Attack Path 1. An attacker creates a ZIP archive with a single apparent root directory. 2. The archiv ...[truncated 1078 chars]
- Remediation
- ## Remediation Suggestions 1. Resolve the extraction root and every candidate destination to canonical absolute paths before creating directories or files. 2. Use `os.path.commonpath()` to verify that every resolved destination remains inside the extraction root. 3. Reject absolute paths, drive-qualified paths, UNC paths, empty member names, and any member containing a `..` path component. 4. Validate every archive member before extracting any content so an invalid archive cannot be partially written. 5. Extract files incrementally only after all members pass validation. 6. Add regression tests covering traversal paths, mixed path separators, absolute paths, drive-qualified paths, and valid nested files. Example containment check: ```python root = os.path.realpath(target_path) destination = os.path.realpath(os.path.join(root, relative_path)) try: contained = os.path.commonpath([root, destination]) == root except ValueError: contained = False if not contained: raise ValueError(f"Unsafe ZIP member path: {member}") ``` Apply equivalent validation consistently to both extraction branches.