T09 · Insecure Skill Coding Practices
Warning
- Location
- converter.py:461
- Finding
- Predictable Temporary File Allows Local File Overwrite and Deletion<![CDATA[ ## Vulnerability Details **File Location**: `converter.py`, lines 461–481 **Vulnerability Type**: Predictable and unsafe temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```python tmp_md = str(output_path).replace(".docx", "_tmp.md") with open(tmp_md, "w", encoding="utf-8") as f: f.write(md) cmd = [ "pandoc", tmp_md, "--from", "markdown+pipe_tables+fenced_code_blocks+inline_notes+raw_attribute", "--to", "docx", f"--reference-doc={self.template}", "--output", output_path, ] result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode != 0: raise RuntimeError(f"Pandoc 转换失败:\n{result.stderr}") try: os.unlink(tmp_md) except: pass ``` ### Technical Analysis The Markdown temporary path is generated deterministically by replacing `.docx` in the caller-supplied output path with `_tmp.md`. The file is then opened in write mode without exclusive creation, symlink checks, or verification that it is a newly created regular file. If a file already exists at the derived path, it is truncated and overwritten. If the path is a symbolic link, Python follows the link and overwrites its target using the privileges of the process. After a successful Pandoc conversion, the predictable path is unconditionally removed. This deletes a pre-existing regular file at that path or removes an attacker-created symlink. The cleanup also uses a broad exception handler, suppressing all failures and making unexpected cleanup behavior difficult to detect. Cleanup is not placed in a `finally` block, so the temporary file remains when Pandoc fails. ### Attack Path 1. The attacker determines or influences the output path passed to `DocxConverter.save()`. 2. The attacker derives the temporary path using the same replacement rule. For example, output `/shared/report.docx` produces `/shared/report_tmp.md`. 3. The attacker performs one of the following: - Places a valuable regular file at the derived tem ...[truncated 1272 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Create temporary files with Python's `tempfile` module using unpredictable names and atomic exclusive creation. - Prefer creating the temporary file in a private temporary directory. If it must be near the output, ensure the directory is trusted and not writable by untrusted users. - Store the exact generated path rather than deriving it through string replacement. - Perform cleanup in a `finally` block. - Catch only expected cleanup exceptions, such as `FileNotFoundError`, rather than suppressing every exception. - Validate that the output path has the expected `.docx` suffix and create its parent directory explicitly if required. - Where shared directories cannot be avoided, reject symlinks and verify file metadata before use. Example hardened implementation: ```python import os import tempfile from pathlib import Path output = Path(output_path) if output.suffix.lower() != ".docx": raise ValueError("The output path must use the .docx extension") output.parent.mkdir(parents=True, exist_ok=True) tmp_path = None try: with tempfile.NamedTemporaryFile( mode="w", encoding="utf-8", suffix=".md", prefix=".docx-converter-", dir=str(output.parent), delete=False, ) as tmp: tmp.write(md) tmp_path = tmp.name cmd = [ "pandoc", tmp_path, "--from", "markdown+pipe_tables+fenced_code_blocks+inline_notes+raw_attribute", "--to", "docx", f"--reference-doc={self.template}", "--output", str(output), ] result = subprocess.run(cmd, capture_output=True, text=True, check=False) if result.returncode != 0: raise RuntimeError(f"Pandoc conversion failed:\n{result.stderr}") finally: if tmp_path is not None: try: os.unlink(tmp_path) except FileNotFoundError: pass ``` ]]>