T09 · Insecure Skill Coding Practices
- Location
install.py:101- Finding
Archive-Controlled Skill Name Enables Arbitrary Directory Deletion and File Placement
- Content
View full analysis
Vulnerability Details
File Location:
install.py:101-104, 123-153
Vulnerability Type: Unvalidated path construction and path traversal
Risk Level: HighVulnerable Code
python # Extract name name_match = re.search(r'name:\s*(.+)', yaml_content) if name_match: info['name'] = name_match.group(1).strip().strip('"\'') def install_to_wsl(self, skill_dir, skill_name): """Install into WSL2""" target_dir = self.wsl_skills_dir / skill_name print(f"✓ Installing into WSL2: {target_dir}") # Delete the existing skill if target_dir.exists(): shutil.rmtree(target_dir) # Copy shutil.copytree(skill_dir, target_dir) return target_dir def install_to_windows(self, skill_dir, skill_name): """Back up to the Windows desktop""" target_dir = self.windows_skills_dir / skill_name print(f"✓ Backing up to Windows: {target_dir}") # Ensure that the directory exists self.windows_skills_dir.mkdir(parents=True, exist_ok=True) # Delete the existing skill if target_dir.exists(): shutil.rmtree(target_dir) # Copy shutil.copytree(skill_dir, target_dir) return target_dirTechnical Analysis
The destination directory name is read directly from the untrusted
namefield in the archive'sSKILL.md. The value is stripped of quotes but is not validated as a single safe filename component.Python's
pathlibdoes not guarantee that joining an untrusted path to a trusted base keeps the resulting path beneath that base. A name containing parent-directory components, such as../../target, can escape the configured skills directory. Ifskill_nameis an absolute path, the absolute path supersedes the intended base path entirely.The escaped path is passed to
shutil.rmtree()beforeshutil.copytree()is called. Consequently, an attacker-controlled package can cause the installer to recursively delete an existing directory and then place package-controlled files at the ...[truncated 2142 chars]- Remediation
View remediation
Remediation Suggestions
- Treat the skill name as a single filename component rather than a path.
- Enforce a strict allowlist, for example:
python SAFE_SKILL_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") if not SAFE_SKILL_NAME.fullmatch(skill_name): raise ValueError("Invalid skill name") - Explicitly reject:
- Absolute paths.
/and\path separators..and..components.- Empty names.
- Platform-specific drive or UNC path syntax.
- Resolve both the trusted base and proposed destination, then verify containment before deletion or copying:
python base = self.wsl_skills_dir.resolve() target = (base / skill_name).resolve() if target.parent != base: raise ValueError("Skill destination escapes the installation directory") - Apply equivalent validation independently to the WSL and Windows destinations.
- Avoid recursively deleting a destination until all archive metadata and paths have passed validation.
- Consider installing into a newly created staging directory and using an atomic rename only after validation succeeds.
- Refuse unsafe input rather than attempting to remove traversal components automatically, because normalization can conceal malicious intent.