T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/audit_skill.py:164
- Finding
- Unbounded Recursive Directory Traversal Through Symbolic Links## Vulnerability Details **File Location**: `scripts/audit_skill.py:164-173` **Vulnerability Type**: Unrestricted recursive traversal and symbolic-link following **Risk Level**: Medium ```python items = sorted(skill_dir.iterdir(), key=lambda p: (p.is_file(), p.name)) for i, item in enumerate(items): is_last = i == len(items) - 1 connector = "└── " if is_last else "├── " lines.append(f"{indent}{connector}{item.name}") if item.is_dir(): sub_indent = indent + (" " if is_last else "│ ") lines.append(build_file_tree(item, sub_indent)) ``` ### Technical Analysis The file-tree generator recursively traverses every item for which `Path.is_dir()` returns true. Because `Path.is_dir()` follows symbolic links, a symbolic link to a directory is treated as an ordinary directory. The implementation does not: - Reject symbolic links. - Track directories that have already been visited. - Enforce a maximum recursion depth or item count. - Resolve paths and verify that they remain inside the audited Skill directory. Consequently, an attacker-controlled Skill directory can contain a symbolic link to itself, an ancestor directory, or a large external directory. A cyclic link can cause repeated recursion until Python raises a recursion error or the process exhausts resources. A link to an external directory can make the analyzer enumerate filenames outside the intended project boundary. ### Attack Path 1. An attacker creates a Skill directory containing a symbolic directory link. 2. The link points to the Skill directory itself, one of its ancestors, or a large directory elsewhere on the filesystem. 3. A user invokes `python scripts/audit_skill.py <skill-directory>`. 4. `build_file_tree()` calls `item.is_dir()`, which follows the symbolic link. 5. The function recursively traverses the linked directory without cycle detection or boundary validation. 6. For a cyclic or very larg ...[truncated 889 chars]
- Remediation
- ## Remediation Suggestions 1. Reject symbolic links before testing whether an item is a directory: ```python if item.is_symlink(): lines.append(f"{indent}[symbolic link skipped: {item.name}]") continue ``` 2. Resolve each candidate path and verify that it remains under the resolved Skill root: ```python root = skill_root.resolve() resolved = item.resolve() try: resolved.relative_to(root) except ValueError: raise ValueError(f"Path escapes the Skill directory: {item}") ``` 3. Track visited directories using resolved paths or `(device, inode)` identifiers to prevent cycles and repeated traversal. 4. Add explicit maximum-depth and maximum-entry limits so unusually large but non-cyclic directory trees cannot exhaust resources. 5. Handle traversal errors such as permission failures, broken links, and link loops without terminating the entire audit. 6. Add regression tests covering self-referential links, links to parent directories, links outside the project root, broken links, and deeply nested directory structures.