T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/scaffold_mcp_server.py:144
- Finding
- Unrestricted Output Path Allows Arbitrary File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/scaffold_mcp_server.py`, lines 55-91 and 144 **Vulnerability Type**: Unrestricted file write and unsafe overwrite **Risk Level**: Medium ### Vulnerable Code ```python def render(result: dict, output_path: Path, fmt: str) -> None: output_path.parent.mkdir(parents=True, exist_ok=True) if fmt == "json": output_path.write_text(json.dumps(result, indent=2), encoding="utf-8") return if fmt == "md": lines = [ f"# {result['summary']}", "", f"- status: {result['status']}", "", "## Planned Files", ] for item in result["details"]["file_map"]: lines.append(f"- {item}") lines.extend(["", "## Tools"]) for tool in result["details"]["tools"]: lines.append(f"- {tool['name']}: {tool['description']}") output_path.write_text("\n".join(lines) + "\n", encoding="utf-8") return with output_path.open("w", newline="", encoding="utf-8") as handle: writer = csv.writer(handle) writer.writerow(["name", "description"]) for tool in result["details"]["tools"]: writer.writerow([tool["name"], tool["description"]]) ``` The unchecked path is passed to this function at line 144: ```python render(result, Path(args.output), args.format) ``` ### Technical Analysis The required `--output` argument is converted directly into a `Path` and passed to `render()`. Unlike `scaffold_root`, the output path is not resolved and checked against the current workspace. All supported output formats use operations that create or truncate the destination: - `Path.write_text()` truncates an existing file. - `Path.open("w")` truncates an existing file. - `mkdir(parents=True, exist_ok=True)` creates attacker-selected parent directories where process permissions permit. The write operations also follow symbolic links. Consequently, an output pa ...[truncated 1236 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Resolve `--output` and enforce workspace containment before creating directories or files: ```python output_path = resolve_path_in_workspace( Path(args.output), workspace_root, "output", args.allow_outside_workspace, ) ``` 2. Do not reuse the scaffold override implicitly. Prefer a separate, explicit option such as `--allow-output-outside-workspace` if external report output is genuinely required. 3. Reject symbolic links in the destination and its existing parent components. Where supported, use descriptor-relative operations and no-follow flags to mitigate time-of-check/time-of-use races. 4. Avoid silently truncating existing files. Open destinations in exclusive creation mode (`"x"`) by default and require an explicit `--force` option to overwrite. 5. Revalidate the resolved destination immediately before writing. 6. Write to a securely created temporary file in the validated destination directory and atomically replace the final path only after all checks succeed. ]]>