T09 · Insecure Skill Coding Practices
Warning
- Location
- lib/tables.mjs:98
- Finding
- Unescaped HTML Injection in the Generic Table Renderer<![CDATA[ ## Vulnerability Details **File Location**: `lib/tables.mjs:98, 107, 116, 125-126` **Vulnerability Type**: HTML injection through unescaped renderer inputs **Risk Level**: Medium ### Complete Code Snippet ```js return `<tr style="${bg}${dim}">${cells.map((c, j) => `<td style="${j === 0 ? edge : ""}padding:9px 6px${j === 0 ? " 9px 9px" : ""};text-align:${o.columns[j]?.align === "right" ? "right" : "left"};vertical-align:top">${c}</td>`).join("")}</tr>`; ``` ```js <div style="font-size:16px;font-weight:700">${o.title}</div> ``` ```js ${o.callout.body} ``` ```js ${o.legend?.length ? `<div style="margin-top:13px;padding-top:11px;border-top:1px solid ${T.line};display:flex;gap:16px;flex-wrap:wrap;align-items:center;color:${T.faint};font-size:10.5px">${o.legend.join("")}</div>` : ""} ${o.footnote ? `<div style="margin-top:9px;color:${T.faint};font-size:11px">${o.footnote}</div>` : ""} ``` The documented local-preview workflow in `SKILL.md:126-130` also opens the generated document in a browser without its sandbox: ```bash node examples/demo.mjs | sed '1d;$d' > /tmp/t.html printf '<!doctype html><meta charset=utf-8><body style="margin:0;padding:18px;background:#1a1410">' > /tmp/p.html cat /tmp/t.html >> /tmp/p.html google-chrome --headless --disable-gpu --no-sandbox --window-size=1100,780 --screenshot=/tmp/t.png file:///tmp/p.html ``` ### Technical Analysis The renderer applies HTML escaping to several values, including metadata, subtitles, column labels, chip text, and numeric values. However, it directly interpolates the following inputs into the generated document: - `o.title` - Every value in `o.rows` - `o.callout.body` - Every value in `o.legend` - `o.footnote` Although some fields are intended to support pre-rendered markup, the API does not establish a security boundary between trusted markup and untrusted data. In particular, `title` is documented as an ordinary string but is not escaped. If any of these fields contains attacker-controll ...[truncated 2777 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Escape all plain-text inputs, including `o.title`, before interpolation: ```js <div style="font-size:16px;font-weight:700">${esc(o.title)}</div> ``` 2. Make row cells plain text by default. Do not require callers to remember which values need escaping. 3. Separate trusted markup from untrusted text through an explicit API. For example, use a dedicated trusted-fragment type created only by helper functions such as `chip()`, `bar()`, `num()`, and `absent()`. 4. Treat callout bodies, legends, and footnotes as text unless rich formatting is explicitly requested. Where rich HTML is necessary, sanitize it with a strict allowlist that rejects: - `<script>`, `<iframe>`, `<object>`, and `<embed>` - Event-handler attributes such as `onerror` and `onclick` - Dangerous URL schemes such as `javascript:` - Remote resource URLs unless they are expressly required - CSS capable of hiding, overlaying, or impersonating trusted output 5. Remove `--no-sandbox` from the documented Chrome command. Run previews in a sandboxed browser and, where feasible, in a container or process with network access disabled. 6. Add security tests for every public renderer field using payloads such as: ```html <img src="https://attacker.invalid/collect" onerror="alert(1)"> ``` Tests should verify that the payload is encoded as visible text or removed by sanitization rather than interpreted as markup. 7. Document the trust model clearly, including which fields accept plain text, which accept trusted renderer fragments, and why raw HTML from external datasets must never be passed directly. ]]>