T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/scan.py:72
- Finding
- Scan-Boundary Bypass Through Symbolic-Link File Traversal<![CDATA[ ## Vulnerability Details **File Location**: `scripts/scan.py:8-9` and `scripts/scan.py:72-77` **Vulnerability Type**: Unrestricted symbolic-link file access **Risk Level**: Medium ### Vulnerable Code ```python def scan_file(filepath, rules): findings = [] try: with open(filepath, "r", encoding="utf-8", errors="ignore") as f: lines = f.readlines() ``` ```python for root, dirs, fnames in os.walk(skill_path): dirs[:] = [d for d in dirs if d not in ("node_modules", ".git", "__pycache__")] for fn in fnames: if os.path.splitext(fn)[1] in exts: files.append(os.path.join(root, fn)) ``` Matched file content is also retained in the returned findings: ```python findings.append({ "rule_id": rule["id"], "rule_name": rule["name"], "severity": rule["severity"], "category": rule["category"], "file": filepath, "line": i, "matched": line.strip()[:120], "description": rule["description"] }) ``` ### Technical Analysis The scanner is designed to process untrusted Skill directories. It enumerates files according to their apparent filename extensions and subsequently opens each path without rejecting symbolic links or checking the canonical path against the canonical scan root. Python's `open()` follows file symbolic links. Consequently, an attacker-controlled Skill can contain a file such as `external.json` that is a symbolic link to a readable file outside the supplied Skill directory. Because the extension check examines the symlink's name rather than its resolved target, the external target is accepted and read. The scanner uses `readlines()` without a file-size or line-length limit. A symlink to a very large readable file—or an ordinary oversized file inside the Skill—can therefore consume excessive memory. Lines matching a rule are also copied into the `matched` field of the returned findings. The current command-line output does not print that field, which limits direct ...[truncated 1967 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Resolve the scan root once and require every candidate's canonical path to remain beneath it: ```python scan_root = os.path.realpath(skill_path) candidate = os.path.join(root, fn) resolved = os.path.realpath(candidate) try: if os.path.commonpath([scan_root, resolved]) != scan_root: continue except ValueError: continue ``` 2. Explicitly reject symbolic-link files before opening them: ```python if os.path.islink(candidate): continue ``` 3. Reduce time-of-check/time-of-use exposure by opening files with platform-appropriate no-follow semantics where available, such as `os.open()` with `os.O_NOFOLLOW`, and then reading through the returned descriptor. 4. Enforce a maximum file size using `os.stat()` or `os.fstat()` and skip files exceeding a documented limit. 5. Stream input rather than loading the entire file: ```python with open(resolved, "r", encoding="utf-8", errors="ignore") as f: for line_number, line in enumerate(f, 1): ... ``` 6. Apply a maximum line length before regex processing to reduce memory consumption and regex-processing abuse. 7. Avoid returning raw matched content unless required. If programmatic consumers need evidence, redact likely secrets and make content inclusion an explicit option. 8. Add tests covering symlinks to files outside the root, broken symlinks, oversized files, cross-filesystem paths, and paths whose textual prefix resembles—but is not contained by—the scan root. ]]>