T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/md_to_xmind.py:135
- Finding
- Predictable Shared Temporary Directory Enables Symlink-Based File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/md_to_xmind.py`, lines 135–160 **Vulnerability Type**: Predictable and insecure temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```python # Create temporary directory temp_dir = "/tmp/xmind_temp" os.makedirs(temp_dir, exist_ok=True) # Save JSON files to temporary directory with open(os.path.join(temp_dir, "manifest.json"), 'w', encoding='utf-8') as f: json.dump(manifest, f, ensure_ascii=False, indent=2) with open(os.path.join(temp_dir, "content.json"), 'w', encoding='utf-8') as f: json.dump(content, f, ensure_ascii=False, indent=2) with open(os.path.join(temp_dir, "metadata.json"), 'w', encoding='utf-8') as f: json.dump(metadata, f, ensure_ascii=False, indent=2) # Create XMind file (ZIP format) print(f"正在保存XMind文件: {xmind_file_path}") with zipfile.ZipFile(xmind_file_path, 'w', zipfile.ZIP_DEFLATED) as xmind_zip: xmind_zip.write(os.path.join(temp_dir, "manifest.json"), "manifest.json") xmind_zip.write(os.path.join(temp_dir, "content.json"), "content.json") xmind_zip.write(os.path.join(temp_dir, "metadata.json"), "metadata.json") # Clean up temporary files import shutil shutil.rmtree(temp_dir) ``` ### Technical Analysis The converter stores intermediate files in the fixed, globally predictable directory `/tmp/xmind_temp`. It creates that directory with `exist_ok=True` but does not verify: - Whether the directory was created by the current process. - Whether it is owned by the expected user. - Whether its permissions prevent access by other local users. - Whether any intermediate file is a symbolic link. - Whether another conversion process is using the same directory. The subsequent calls to `open(..., "w")` follow symbolic links. A local attacker who can prepare `/tmp/xmind_temp` before the converter runs could place `manifest.json`, `content.json`, or `metadata.json` as a symbolic link to another file writable by the converter's user. When the co ...[truncated 2355 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Replace the fixed directory with a unique, securely created temporary directory: ```python import tempfile with tempfile.TemporaryDirectory(prefix="xmind_") as temp_dir: manifest_path = os.path.join(temp_dir, "manifest.json") content_path = os.path.join(temp_dir, "content.json") metadata_path = os.path.join(temp_dir, "metadata.json") with open(manifest_path, "w", encoding="utf-8") as f: json.dump(manifest, f, ensure_ascii=False, indent=2) with open(content_path, "w", encoding="utf-8") as f: json.dump(content, f, ensure_ascii=False, indent=2) with open(metadata_path, "w", encoding="utf-8") as f: json.dump(metadata, f, ensure_ascii=False, indent=2) with zipfile.ZipFile( xmind_file_path, "w", zipfile.ZIP_DEFLATED ) as xmind_zip: xmind_zip.write(manifest_path, "manifest.json") xmind_zip.write(content_path, "content.json") xmind_zip.write(metadata_path, "metadata.json") ``` This hardening provides a randomly named directory created atomically with restrictive permissions and automatically cleans up only the files belonging to the current invocation. Additional safeguards should include: 1. Avoid all fixed names under globally writable locations such as `/tmp`. 2. Keep temporary resources inside a context manager so cleanup also occurs after exceptions. 3. Ensure each conversion invocation has an isolated workspace. 4. Validate the output path and reject output files that resolve to symbolic links when the caller is not fully trusted. 5. When stronger protection is required, create output files atomically with exclusive creation semantics and rename them into place only after successful archive generation. ]]>