T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/render_puzzle.py:42
- Finding
- Arbitrary HTML and JavaScript Injection During Puzzle Rendering<![CDATA[ ## Vulnerability Details **File Location**: `scripts/render_puzzle.py:42-47`; injection sinks in `assets/puzzle-template.html:145-151` **Vulnerability Type**: Unescaped data embedded into executable HTML and an HTML attribute **Risk Level**: High ### Vulnerable Code `scripts/render_puzzle.py:42-47`: ```python def build_html(template_path: str, puzzle: dict, cartoon_filename: str) -> str: with open(template_path) as f: html = f.read() html = html.replace("__PUZZLE_JSON__", json.dumps(puzzle)) html = html.replace("__CARTOON_IMAGE__", cartoon_filename) return html ``` `assets/puzzle-template.html:145-151`: ```html <div class="layout"> <div class="puzzle-col" id="puzzle-area"></div> <div class="cartoon-col" id="cartoon-col"> <img src="__CARTOON_IMAGE__" alt="Puzzle hint illustration" /> </div> </div> <script> const puzzle = __PUZZLE_JSON__; ``` ### Technical Analysis `build_html()` performs direct string substitution into two distinct HTML parsing contexts: 1. Puzzle JSON is inserted directly into an executable `<script>` element. 2. The image filename is inserted directly into a double-quoted HTML attribute. `json.dumps()` produces valid JSON but does not make the result safe for insertion into an HTML script element. In particular, an attacker-controlled string containing `</script>` can terminate the surrounding script because HTML parsing recognizes the closing tag even when it appears inside a JavaScript string. The remainder of the value can then introduce a new script or arbitrary HTML. Likewise, `os.path.basename()` used by the caller removes directory components but does not remove quotation marks or HTML metacharacters. A crafted image filename can close the `src` attribute and inject additional attributes or elements. The application subsequently serves the generated file over localhost and directs a browser to load it. Therefore, injected active content executes in the rendering browser's security ...[truncated 2094 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not place untrusted serialized data directly into an executable script element. Prefer writing the puzzle to a separate JSON file and retrieving it as data: ```javascript const puzzle = await fetch('./puzzle.json').then(response => response.json()); ``` 2. If inline JSON is required, place it in a non-executable element: ```html <script id="puzzle-data" type="application/json">...</script> ``` Before insertion, escape characters significant to HTML parsing, including at minimum: - `<` as `\u003c` - `>` as `\u003e` - `&` as `\u0026` - U+2028 and U+2029 where relevant Parse the element's text as JSON rather than evaluating it as JavaScript. 3. Do not interpolate the image filename into markup. Create the image element with a fixed template and assign the filename through a DOM property after validating it, or apply context-specific HTML attribute escaping. 4. Restrict accepted image filenames to a conservative allowlist, such as: ```text ^[A-Za-z0-9._-]+$ ``` Reject quotes, angle brackets, control characters, URL schemes, and path separators. 5. Add a restrictive Content Security Policy that blocks inline and remote scripts. For example, move legitimate JavaScript to a static file and use: ```text Content-Security-Policy: default-src 'none'; script-src 'self'; img-src 'self'; style-src 'self' ``` This should be treated as defense in depth rather than a substitute for correct encoding. 6. Add regression tests using clue values containing `</script>`, quotes, angle brackets, ampersands, and Unicode separators, as well as image filenames containing quotes and markup. ]]>