T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/write.py:263- Finding
Preset path traversal can disclose arbitrary local Markdown files
- Content
View full analysis
Path | None: for root in preset_dirs: d = root / subdir if not d.exists(): continue for ext in exts: p = d / f"{name}{ext}" if p.exists(): return p return None def _load_closing_block(screening: dict, article_cfg: dict) -> str: """ Closing block: preset selection reads default_closing_block from article.yaml. If no preset is selected, use the inline closing_block from merged context. """ default_name = _coerce_single_preset( "default_closing_block", article_cfg.get("default_closing_block"), ) if default_name: preset_dirs = _preset_dirs(_aws_root()) found = _find_preset_file( preset_dirs, "closing-blocks", default_name, [".md"], ) if found: _info(f"Loading closing block preset: {found}") return found.read_text(encoding="utf-8") return (screening.get("closing_block") or "").strip() ``` The same unsafe lookup is also used by `_load_structure_template()` for `default_structure`. ### Technical Analysis Preset names read from the article's `article.yaml` are appended directly to a preset directory: ```python p = d / f"{name}{ext}" ``` The code does not reject: - `..` path components; - forward or backward path separators; - absolute paths; - paths that resolve outside the intended preset directory. It also does not resolve the resulting candidate and verify that the candidate remains a descendant of `presets/structures` or `presets/closing-blocks`. For example, a preset value conceptually equivalent to ...[truncated 2006 chars]- Remediation
View remediation
Path | None: if ( not name or Path(name).is_absolute() or ".." in Path(name).parts or "/" in name or "\\" in name or not _SAFE_PRESET_NAME.fullmatch(name) ): _err("Preset name contains prohibited path characters") root = preset_dir.resolve() candidate = (root / f"{name}{ext}").resolve() try: candidate.relative_to(root) except ValueError: _err("Preset path escapes the permitted preset directory") return candidate if candidate.is_file() else None ``` Where supported, `candidate.is_relative_to(root)` can be used instead of the `relative_to()` exception pattern. ]]>