T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/docxtpl-render-batch.py:80
- Finding
- CSV-Controlled Path Traversal in Batch Output Filenames## Vulnerability Details **File Location**: `scripts/docxtpl-render-batch.py`, lines 80–100 **Vulnerability Type**: Path traversal and arbitrary file write **Risk Level**: High ### Vulnerable Code ```python for i, row in enumerate(rows): base_name = row[args.id_column].strip() if not base_name: print(f"Warning: Row {i} has empty {args.id_column}, skipping", file=sys.stderr) skipped += 1 continue output_path = out_dir / f"{base_name}{args.suffix}.docx" if output_path.exists() and not args.overwrite: print(f"Skip (exists): {output_path}", file=sys.stderr) skipped += 1 continue if args.dry_run: print(f"[DRY RUN] Would generate: {output_path}") generated += 1 continue tpl.render(row, autoescape=args.autoescape) tpl.save(str(output_path)) tpl.reset_replacements() ``` ### Technical Analysis The value used as `base_name` comes directly from the attacker-controllable CSV or TSV ID column. It is combined with the output directory without validating filename separators, parent-directory components, or absolute paths. A value such as `../../outside/report` causes the resulting path to escape the intended output directory. An absolute identifier can cause `pathlib` to disregard the configured output directory entirely. The application does not resolve the final path and verify that it remains beneath `out_dir`. The `--overwrite` option increases the impact by allowing an existing writable target to be replaced. Without that option, the script can still create a new file at an unintended writable location if the destination and its parent directories are available. ### Attack Path 1. An attacker creates or modifies the CSV/TSV data file used for batch rendering. 2. The attacker places a traversal path or absolute path in the configured ID column, for example: ```csv id,name ../../outside/report,Alice ``` 3. A user runs `docxtpl-render-batch.py` with ...[truncated 965 chars]
- Remediation
- ## Remediation Suggestions 1. Treat the ID column as a filename identifier rather than a path: - Reject absolute paths. - Reject `/`, `\`, `..`, null bytes, and platform-specific path separators. - Allow only a conservative character set such as letters, digits, underscores, and hyphens. 2. Resolve and validate the final output path before writing: ```python import re safe_id_pattern = re.compile(r"^[A-Za-z0-9_-]+$") base_name = row[args.id_column].strip() if not safe_id_pattern.fullmatch(base_name): raise ValueError(f"Unsafe output identifier: {base_name!r}") root = out_dir.resolve() output_path = (root / f"{base_name}{args.suffix}.docx").resolve() try: output_path.relative_to(root) except ValueError: raise ValueError("Output path escapes the configured output directory") ``` 3. Validate `args.suffix` under the same filename policy because it also contributes to the resulting path. 4. Perform containment validation regardless of whether `--overwrite` is enabled. 5. Prefer exclusive creation for non-overwrite operations and avoid relying solely on a separate `exists()` check, which can introduce a time-of-check/time-of-use race. 6. Add tests covering parent traversal, absolute paths, alternate separators, symbolic links, and valid identifiers.