T09 · Insecure Skill Coding Practices
Error
- Location
- render_chart.py:316
- Finding
- Arbitrary File Write Through Unsanitized Output Stem<![CDATA[ ## Vulnerability Details **File Location**: `render_chart.py`, lines 316–320 and 354 **Vulnerability Type**: Path traversal and unrestricted output path **Risk Level**: High ### Vulnerable Code ```python stem = output_stem or f"{symbol.lower()}_{period}m_market_chart_renderer" base = OUTDIR / stem json_path = base.with_suffix('.json') html_path = base.with_suffix('.html') png_path = base.with_suffix('.png') ``` The value is supplied directly through a command-line argument: ```python parser.add_argument('--output-stem', default=None) ``` The resulting paths are subsequently written without containment validation: ```python html_path.write_text(build_html(payload), encoding='utf-8') json_path.write_text( json.dumps( {'chart_input': chart_input, 'payload': payload}, ensure_ascii=False, indent=2, ), encoding='utf-8', ) ``` ### Technical Analysis The user-controlled `output_stem` is combined with `OUTDIR` without rejecting absolute paths, parent-directory components, or path separators. Python's `pathlib` discards the left operand when the right operand is absolute. Therefore, an absolute `output_stem` causes `base` to resolve outside `OUTDIR`. A relative value containing `../` can also escape the intended output directory. Calling `with_suffix()` changes the final suffix but does not prevent traversal or restore containment. The application then writes HTML and JSON directly to the resulting paths. When PNG generation is enabled, the path is also passed to Chrome through `--screenshot`. ### Attack Path 1. An attacker obtains control over the command-line arguments used to launch the renderer. 2. The attacker supplies an absolute or traversing output stem, for example: ```bash python3 render_chart.py \ --symbol MA0 \ --period 60 \ --no-png \ --output-stem ../../attacker-selected/location ``` 3. `OUTDIR / output_stem` resolves outside the documented image output directory. 4. `wi ...[truncated 872 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Restrict `output_stem` to a simple filename stem rather than accepting a path. - Reject absolute paths, `..`, path separators, NUL characters, and unexpected filename characters. - Resolve both the output directory and candidate destination, then verify containment before writing: ```python import re def safe_output_base(output_stem: str) -> Path: if not re.fullmatch(r'[A-Za-z0-9._-]+', output_stem): raise ValueError('output stem contains unsupported characters') root = OUTDIR.resolve() candidate = (root / output_stem).resolve() if candidate.parent != root: raise ValueError('output path escapes the output directory') return candidate ``` - Apply the containment check independently to every final path before writing. - Consider exclusive file creation or an explicit overwrite option if existing files must not be replaced silently. - Run the renderer under a dedicated, least-privileged account with write access limited to the intended output directory. ]]>