T09 · Insecure Skill Coding Practices
- Location
scripts/split_skills.py:138- Finding
Symlink-Based Write Outside the Skill References Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/split_skills.py, lines 138–152 and 167–170
Vulnerability Type: Symlink following leading to arbitrary file overwrite
Risk Level: HighTechnical Analysis
The script derives output filenames from headings in a skill's
SKILL.md. During collision checking, it usesPath.exists():python for i, ch in enumerate(chunks, 1): base = slug(ch[0][0]) + (f"-{i}" if i > 1 else "") target, n = refs / f"{base}.md", 2 body = "".join(t for _, t in ch) # unique name within the run: two sections with the SAME heading must not # collapse into one file with one table-of-contents entry while target.name in used_names or (target.exists() and target.read_text(encoding="utf-8") != body): target = refs / f"{base}-{n}.md" n += 1 used_names.add(target.name) span = f"{ch[0][0][:40]} … {ch[-1][0][:40]}" if len(ch) > 1 else ch[0][0][:60] toc.append(f"* `references/{target.name}` — {span}\n") files.append((target.name, body))The selected destinations are subsequently written without checking for symbolic links or verifying that their resolved paths remain inside the intended
referencesdirectory:python refs.mkdir(parents=True, exist_ok=True) for name, body in files: (refs / name).write_text(body, encoding="utf-8") skill_md.write_text(new_md, encoding="utf-8")Path.exists()returns false when a path is a dangling symbolic link. Consequently, a dangling link at a predicted generated filename is treated as an unused destination.Path.write_text()then follows the symbolic link and creates or overwrites its target.The generated filename is predictable because it is derived from an attacker-controlled H2 heading through
slug(). The script does not resolve the destination, enforce containment, reject symbolic links, or use a no-follow file-crea ...[truncated 1616 chars]- Remediation
View remediation
Remediation Suggestions
- Reject symbolic links for the skill directory,
referencesdirectory, and every destination component before writing. - Resolve the intended output directory and candidate destination, then verify containment:
python refs_real = refs.resolve(strict=True) destination = refs / name resolved_parent = destination.parent.resolve(strict=True) if resolved_parent != refs_real: raise ValueError("Output path escapes the references directory") - Create output files using descriptor-based, no-follow semantics, such as
os.open()withO_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, and write through the returned descriptor. Handle platforms withoutO_NOFOLLOWexplicitly rather than silently falling back. - Do not rely on a separate
exists()check before writing because that also creates a time-of-check/time-of-use race. Perform validation and exclusive creation as one atomic operation. - Open the
referencesdirectory as a trusted directory descriptor and use directory-relative operations where supported, preventing path replacement between validation and creation. - Add regression tests covering dangling symlinks, links to existing external files, replacement of
referenceswith a symlink, and concurrent destination replacement. The expected behavior should be a safe failure without modifying any external target.
- Reject symbolic links for the skill directory,