T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/package_skill.py:114
- Finding
- Skill Packager Can Disclose Files Outside the Selected Directory Through Symbolic Links## Vulnerability Details **File Location**: `scripts/package_skill.py:114-118` **Vulnerability Type**: Symbolic-link dereferencing and insufficient archive boundary validation **Risk Level**: Medium ### Vulnerable Code ```python files = [p for p in skill_path.rglob('*') if p.is_file() and not _should_exclude(p)] for file_path in sorted(files): # Calculate the relative path within the zip arcname = file_path.relative_to(skill_path.parent) zipf.write(file_path, arcname) print(f" Added: {arcname}") ``` ### Technical Analysis The packager recursively selects entries for which `Path.is_file()` returns true. A symbolic link targeting a regular file can satisfy this test. The code does not call `is_symlink()` and does not verify that the resolved target remains under the resolved Skill directory. `file_path.relative_to(skill_path.parent)` validates only the lexical path used as the archive member name. It does not validate the location of the resolved file target. When `zipf.write()` opens the path, it can read the external target and store its contents under the symbolic link's apparent in-tree name. The existing validation functions do not reject symbolic links or scan the package for sensitive files. Consequently, successful structural validation does not mitigate this issue. ### Attack Path 1. An attacker supplies or modifies a Skill directory that otherwise passes structural validation. 2. The attacker creates an in-tree symbolic link, such as `references/report.md`, targeting a readable file outside the Skill directory. 3. A user runs `scripts/package_skill.py` against the affected Skill. 4. The symbolic link passes the `p.is_file()` filter. 5. `zipf.write()` reads the external target and adds its contents to the `.skill` archive as an apparently legitimate in-tree file. 6. The user distributes or uploads the archive, unintentionally disclosing the external file. ### Impact Assessment Ex ...[truncated 389 chars]
- Remediation
- ## Remediation Suggestions 1. Reject symbolic links explicitly before packaging: ```python if file_path.is_symlink(): raise ValueError(f"Symbolic links are not permitted: {file_path}") ``` 2. Resolve every candidate and verify that it remains under the approved root: ```python root = skill_path.resolve() for file_path in sorted(skill_path.rglob("*")): if file_path.is_symlink(): raise ValueError(f"Symbolic links are not permitted: {file_path}") if not file_path.is_file() or _should_exclude(file_path): continue resolved = file_path.resolve(strict=True) try: resolved.relative_to(root) except ValueError: raise ValueError(f"File escapes skill directory: {file_path}") arcname = Path(skill_path.name) / file_path.relative_to(root) zipf.write(resolved, arcname) ``` 3. Reject symlinked directories as well as file symlinks, rather than relying on traversal behavior that may vary by runtime or implementation. 4. Add package-time checks for common sensitive artifacts such as `.env`, private keys, credential files, and hidden configuration files. 5. Present the complete package manifest and require confirmation before producing a distributable archive. 6. Add regression tests covering: - A symlink to an external regular file. - A symlink to an external directory. - Broken symbolic links. - Nested links. - Legitimate files whose names resemble excluded paths.