T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_xmind.py:287
- Finding
- Arbitrary File Overwrite Through an Unvalidated Output Path<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate_xmind.py`, lines 287 and 312–318 **Vulnerability Type**: Arbitrary file overwrite / path traversal **Risk Level**: Medium ### Vulnerable Code ```python # scripts/generate_xmind.py:287 with zipfile.ZipFile(output_file, 'w', zipfile.ZIP_DEFLATED) as zf: zf.writestr('content.xml', content_xml) zf.writestr('styles.xml', styles_xml) zf.writestr('comments.xml', comments_xml) zf.writestr('META-INF/manifest.xml', manifest_xml) ``` The skill entry point passes the caller-controlled path directly to this file-writing operation: ```python # scripts/generate_xmind.py:305-318 def run(input_data): """ Skill entry point invoked by OpenClaw. """ test_data = input_data.get("test_data", DEFAULT_TEST_DATA) output_file = input_data.get("output_file", "测试用例.xmind") if isinstance(test_data, str): test_data = json.loads(test_data) result = generate_xmind(test_data, output_file) return result ``` The command-line interface exposes the same behavior: ```python # scripts/generate_xmind.py:325-335 output_file = sys.argv[2] if len(sys.argv) > 2 else "测试用例.xmind" if input_file: with open(input_file, 'r', encoding='utf-8') as f: test_data = json.load(f) else: test_data = DEFAULT_TEST_DATA result = generate_xmind(test_data, output_file) ``` ### Technical Analysis The `output_file` value is used without path normalization, directory confinement, extension validation, symlink checks, or overwrite authorization. Python's `zipfile.ZipFile` with mode `'w'` creates the specified file or truncates an existing file before writing the XMind ZIP archive. Consequently, a caller can supply: - An absolute path. - A relative path containing parent-directory traversal components. - A path to an existing writable file. - Potentially a path resolving through a symbolic link. Although `output_file` is not declared in `skill.yaml`, the Python `run()` func ...[truncated 2082 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Create a dedicated output directory controlled by the skill and resolve all output paths relative to it. 2. Reject absolute paths and verify that the normalized destination remains inside the approved directory: ```python from pathlib import Path OUTPUT_DIR = Path("outputs").resolve() OUTPUT_DIR.mkdir(parents=True, exist_ok=True) requested_name = Path(output_file) if requested_name.is_absolute(): raise ValueError("Absolute output paths are not permitted") destination = (OUTPUT_DIR / requested_name).resolve() if OUTPUT_DIR not in destination.parents: raise ValueError("Output path escapes the approved directory") if destination.suffix.lower() != ".xmind": raise ValueError("Output file must use the .xmind extension") ``` 3. Prefer accepting only a basename rather than an arbitrary path. 4. Reject existing targets by opening with exclusive creation semantics, or require a separate explicit overwrite option. 5. Check parent directories and target components for symbolic links before writing. Where feasible, use platform-supported no-follow file operations. 6. Generate the archive in a securely created temporary file within the approved directory, then atomically rename it to the validated destination. 7. Add `output_file` to `skill.yaml` only if caller selection is required. Constrain it with a filename-only pattern and document its security restrictions. Otherwise, remove caller control and always generate a server-assigned filename. 8. Run the skill under a least-privileged account with write access limited to its designated output directory. 9. Add tests covering absolute paths, parent traversal, nested traversal, symlink targets, existing files, invalid extensions, and normal output generation. ]]>