T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/generate_document.py:104- Finding
Caller-Controlled Output Path Allows Arbitrary File Overwrite
- Content
View full analysis
Vulnerability Details
File Location:
scripts/generate_document.py:35-43,scripts/generate_document.py:49-70, andscripts/generate_document.py:104-131
Vulnerability Type: Unrestricted file write and arbitrary file overwrite
Risk Level: MediumVulnerable Code
The output path is accepted directly from a command-line argument:
python # Generate or rewrite mode template_id = sys.argv[1] output_path = sys.argv[2]The caller-controlled path reaches truncating file-write operations without validation or confinement:
python def generate_template(template_data, output_path): """Generate a blank template document""" print(f"[INFO] Generating template to {output_path}...") # Simplified: just write the template content as text with open(output_path, 'w', encoding='utf-8') as f: f.write(f"# {template_data.get('title_zh', '')} / {template_data.get('title_en', '')}\n\n") f.write(f"Clause: {template_data.get('clause', '')}\n\n") for section in template_data.get("sections", []): f.write(f"## {section.get('heading_zh', '')} / {section.get('heading_en', '')}\n") f.write(f"{section.get('content_zh', '')} / {section.get('content_en', '')}\n\n")The rewrite path contains the same unsafe sink:
python def rewrite_document(parsed_data, template_data, gaps, output_path): """Rewrite existing document based on template and gap analysis""" print(f"[INFO] Rewriting document to {output_path}...") # Simplified: merge parsed data with template with open(output_path, 'w', encoding='utf-8') as f: f.write(f"# {template_data.get('title_zh', '')} / {template_data.get('title_en', '')}\n\n") f.write(f"Based on: {parsed_data.get('structure', {}).get('file_name', 'Unknown')}\n\n") for section in template_data.get("sections", []): f.write(f" ...[truncated 3412 chars]- Remediation
View remediation
Remediation Suggestions
- Define a dedicated, minimally privileged output directory and resolve all requested output names relative to it.
- Canonicalize both the output root and candidate path with
pathlib.Path.resolve(), then verify that the candidate remains beneath the approved root. - Reject absolute paths, traversal components, device paths, and unexpected file extensions.
- Prevent silent replacement by opening new files in exclusive creation mode (
x) or requiring explicit, trusted overwrite authorization. - Defend against symbolic-link attacks by rejecting symlink targets and, where supported, using operating-system flags such as
O_NOFOLLOW. - Run the Skill under a dedicated account with write permission only to its output directory.
- Use atomic creation in the approved directory and rename the completed file into place only after successful generation.
- Add tests covering absolute paths,
../traversal, existing-file collisions, symbolic links, and paths that resolve outside the output root.
Example confinement logic:
python from pathlib import Path OUTPUT_ROOT = Path("generated_documents").resolve() def safe_output_path(requested_name): candidate_input = Path(requested_name) if candidate_input.is_absolute() or ".." in candidate_input.parts: raise ValueError("Invalid output path") candidate = (OUTPUT_ROOT / candidate_input).resolve() if candidate != OUTPUT_ROOT and OUTPUT_ROOT not in candidate.parents: raise ValueError("Output path escapes the approved directory") if candidate.suffix.lower() not in {".txt", ".md", ".docx"}: raise ValueError("Unsupported output extension") candidate.parent.mkdir(parents=True, exist_ok=True) return candidateThe returned path should then be created exclusively or passed through an explicit trusted overwrite policy.