T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/run.py:31
- Finding
- Directory Scan Boundary Bypass Through Symbolic Links<![CDATA[ ## Vulnerability Details **File Location**: `scripts/run.py:31-39` and `scripts/run.py:136-149` **Vulnerability Type**: Unrestricted symbolic-link traversal and unintended local file disclosure **Risk Level**: Medium ### Vulnerable Code ```python def list_text_files(root: Path, limit: int = 50): results = [] for path in root.rglob("*"): if len(results) >= limit: break if path.is_file(): if path.suffix.lower() in {".md",".txt",".json",".yaml",".yml",".py",".js",".ts",".csv",".tsv",".sh"}: results.append(path) return results ``` ```python def pattern_report(spec: dict, path: Path, limit: int) -> str: targets = [path] if path.is_file() else list_text_files(path, limit=limit) findings = [] for target in targets: text = read_text(target) for name, pattern in PATTERNS.items(): for match in re.finditer(pattern, text, flags=re.IGNORECASE): snippet = match.group(0) if "secret_like" == name: snippet = re.sub(r"([A-Za-z0-9_\-]{4})[A-Za-z0-9_\-]+", r"\1***", snippet) findings.append((str(target), name, snippet[:160])) ``` ### Technical Analysis The directory scanner recursively discovers entries with `Path.rglob()` and accepts them when `Path.is_file()` returns true. Both `is_file()` and the subsequent `read_text()` operation follow symbolic links. The implementation does not reject symbolic links or resolve each candidate and verify that its canonical path remains inside the requested scan root. Consequently, a directory supplied by an untrusted party can contain a symbolic link whose filename has an accepted extension but whose target is outside the directory. The scanner will read the external target using the operating-system permissions of the process running the Skill. The built-in pattern scanner only includes matched portions of a file in its report, which limits disclosure, ...[truncated 1652 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject symbolic links before accepting a scan target: ```python if path.is_symlink(): continue ``` 2. Resolve the scan root and every candidate, then enforce containment: ```python root_resolved = root.resolve() for path in root.rglob("*"): if path.is_symlink(): continue try: resolved = path.resolve(strict=True) resolved.relative_to(root_resolved) except (OSError, ValueError): continue if resolved.is_file() and resolved.suffix.lower() in ALLOWED_SUFFIXES: results.append(resolved) ``` 3. Apply the same boundary validation when the direct `--input` value is a file, because `targets = [path] if path.is_file()` also follows a symbolic link. 4. Where supported, open files using operating-system facilities that prevent following symbolic links, such as `O_NOFOLLOW`. This reduces time-of-check/time-of-use race conditions in which a validated file is replaced by a symbolic link before it is opened. 5. Catch resolution and file-access exceptions so broken links, permission failures, and concurrent filesystem changes result in a controlled warning rather than an unexpected termination. 6. Add automated tests covering: - A symbolic link to a file outside the scan root. - A symbolic link to a file inside the scan root. - A broken symbolic link. - Replacement of a validated file during scanning. - A direct `--input` path that is itself a symbolic link. 7. Run scans under a least-privileged account so that any remaining filesystem traversal issue cannot access unrelated sensitive files. ]]>