T09 · Insecure Skill Coding Practices
Error
- Location
- src/index.py:449
- Finding
- Path Traversal Enables Out-of-Scope File Access and Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `src/index.py:449-454`, with the unsafe write occurring at `src/index.py:444` **Vulnerability Type**: Unrestricted filesystem paths and path traversal **Risk Level**: High ### Vulnerable Code ```python prs.save(str(output_path)) ``` ```python async def handler(input: dict[str, Any], _context: Any) -> dict[str, Any]: skill_root = Path(__file__).resolve().parent.parent template_file = input.get("template_file") or "PPT_Template.pptx" template_path = skill_root / template_file if not template_path.exists(): raise FileNotFoundError(f"Template not found: {template_path}") mode = (input.get("mode") or "xml").strip().lower() if mode != "xml": raise ValueError("Only xml mode is supported") title = input.get("title", "Corporate Deck") output_filename = input.get("output_filename", "openclaw_generated_xml.pptx") output_path = skill_root / output_filename ``` ### Technical Analysis The `template_file` and `output_filename` parameters are accepted from caller-controlled input and combined with `skill_root` without validation or a resolved-path containment check. A path containing `../` components can escape the skill directory. In addition, when the right-hand operand of a `pathlib.Path` join is absolute, Python discards the preceding `skill_root`. Consequently, both relative traversal paths and absolute paths can select files outside the intended project directory. The template path is passed to `Presentation`, allowing any readable, structurally valid PPTX file accessible to the process to be loaded. The output path is passed directly to `prs.save`, allowing PPTX data to be written to or overwrite any filesystem path writable by the process. The check performed for `template_path` only establishes that the path exists. It does not verify that the resolved path remains under the skill directory, that it is a regular file, or that it has an approved extension. ...[truncated 1709 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject absolute paths for both `template_file` and `output_filename`. 2. Resolve each candidate path before using it and enforce containment within dedicated approved directories: ```python def confined_path(base: Path, supplied: str, extension: str) -> Path: if not isinstance(supplied, str) or not supplied: raise ValueError("A non-empty filename is required") relative = Path(supplied) if relative.is_absolute(): raise ValueError("Absolute paths are not allowed") base = base.resolve() candidate = (base / relative).resolve() try: candidate.relative_to(base) except ValueError as exc: raise ValueError("Path escapes the approved directory") from exc if candidate.suffix.lower() != extension: raise ValueError(f"Only {extension} files are allowed") return candidate ``` 3. Use separate directories for trusted templates and generated output. Do not allow callers to select arbitrary files from the entire skill directory. 4. Restrict template files to an allowlist of known templates where possible. 5. Require the template path to be a regular file and reject symbolic links when the deployment threat model permits untrusted local filesystem changes. 6. Generate server-side output names instead of accepting unrestricted caller-provided paths. 7. Use exclusive file creation or explicit overwrite controls to avoid silently replacing existing files. 8. Run the skill under a dedicated low-privilege account with read access only to approved templates and write access only to a dedicated output directory. ]]>