T09 · Insecure Skill Coding Practices
Warning
- Location
- tools/md2docx.py:188
- Finding
- Predictable Temporary File Allows File Overwrite and Deletion## Vulnerability Details **File Location**: `tools/md2docx.py`, lines 188–192 and 262–264 **Vulnerability Type**: Predictable and non-exclusive temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```python date_line = f"\n\n---\n\n{datetime.now().strftime('%Y年%m月%d日')}" temp_input_file = input_file.with_suffix(input_file.suffix + '.temp.md') try: with open(temp_input_file, 'w', encoding='utf-8') as f: f.write(content + date_line) # The temporary file is subsequently supplied to Pandoc. # ... finally: # Clean up temporary file if temp_input_file.exists(): temp_input_file.unlink() ``` ### Technical Analysis The converter constructs a deterministic temporary path by appending `.temp.md` to the input filename. It then opens that path in write mode without exclusive creation or protection against symbolic links. If the generated path already contains a regular file, the converter truncates and overwrites it. On platforms where symbolic links are supported, an attacker able to modify the input directory can pre-create the predictable path as a symbolic link to another file writable by the converter process. Opening the link in write mode follows it and replaces the target's contents with the generated Markdown. The cleanup operation then removes the predictable directory entry. The existence check before `unlink()` also introduces a time-of-check/time-of-use window. An attacker with concurrent directory access may replace the path between the check and deletion, although deletion normally affects the directory entry rather than the target of a symbolic link. No shell-command injection was identified in the Pandoc invocation because `subprocess.run()` receives an argument list and does not enable a shell. ### Attack Path 1. The victim selects an input such as `/shared/report.md`. 2. The attacker predicts that the converter will use `/shared/report.md.temp.md`. 3. Before conversion, the attacker creates th ...[truncated 1341 chars]
- Remediation
- ## Remediation Suggestions 1. Replace the deterministic filename with a securely and atomically created temporary file using `tempfile.NamedTemporaryFile(delete=False)` or `tempfile.mkstemp()`. 2. Prefer a private, controlled temporary directory rather than the input file's directory. 3. Close and flush the temporary file before invoking Pandoc, particularly for cross-platform compatibility. 4. Retain the exact uniquely generated path and remove only that path in the `finally` block. 5. Avoid separate existence checks before cleanup. Attempt deletion directly and handle `FileNotFoundError`. 6. If a temporary file must be created beside the input, use exclusive creation and platform-appropriate protections against symbolic-link traversal. 7. Add tests covering: - A pre-existing candidate temporary file. - A symbolic link at the candidate path on supported platforms. - Concurrent replacement or removal during cleanup. - Cleanup after Pandoc failure. A safer implementation pattern is: ```python import os import tempfile temp_path = None try: fd, temp_name = tempfile.mkstemp( suffix=".md", prefix=f".{input_file.stem}-", dir=None, text=True, ) temp_path = Path(temp_name) with os.fdopen(fd, "w", encoding="utf-8") as temp_file: temp_file.write(content + date_line) # Invoke Pandoc with temp_path. finally: if temp_path is not None: try: temp_path.unlink() except FileNotFoundError: pass ```