anthropics / anthropics/skills
skill-creator: eval-viewer escapeHtml is not attribute-safe and is applied inconsistently (display-path XSS)
- Dominant language
- Python
- Stars
- 176k
- Forks
- 20.8k
- Avg merge
- 7h 21m
- Merged PRs (30d)
- 5
Description
### Summary
The skill-creator eval-viewer (`skills/skill-creator/eval-viewer/viewer.html`)
assembles HTML by string concatenation and assigns it with `innerHTML` in
`renderGrades` and `renderBenchmark`, relying on a helper `escapeHtml()` at each
interpolation point. Two gaps let attacker-influenced data reach the DOM as
markup:
1. **`escapeHtml()` is not attribute-safe.** It is implemented with the
`textContent` → `innerHTML` round-trip, which escapes `<`, `>`, and `&` but
**not** `"` or `'`. It is nonetheless used inside an HTML **attribute** — the
per-assertion table renders grader evidence into a `title="..."` tooltip
(`renderBenchmark`, the `` line). Evidence
containing a double quote breaks out of the attribute and can attach an event
handler.
2. **Several externally-sourced strings are interpolated raw**, with no
`escapeHtml()` at all: the config label in the per-eval breakdown rows (while
the *same* value is escaped in the table header), and `metadata.timestamp`,
`metadata.evals_run`, `metadata.runs_per_configuration`, and the `delta.*`
cells.
### Why this is reachable
`EMBEDDED_DATA` (from `benchmark.json` / `grading.json`) carries data that is
user-authored (assertion text, eval and skill names) or that quotes the output
of the skill under test (grader "evidence"). None of that is trusted markup.
### Reproduction
1. In a grading result, set an assertion's `evidence` to:
```
x" onmouseover="alert(document.domain)
```
2. Generate the review and open the **Benchmark** tab.
3. Hover the ✓/✗ badge for that assertion in the per-assertion table. The
`title="Run 1: x" onmouseover="alert(document.domain)">` markup closes the
attribute early and the injected `onmouseover` handler fires.
### Fix
- Rewrite `escapeHtml()` to escape `&`, `<`, `>`, `"`, and `'` explicitly, so it
is safe in both element-text and attribute contexts.
- Route the remaining raw externally-sourced string interpolations through
`escapeHtml()`, leaving only locally-computed numbers interpolated directly.
The escaped forms render identically for normal data, so there is no visible
change.
### Scope / related
- This is the **display path**. The **embed path** — `generate_review.py`
injecting `EMBEDDED_DATA` into the inline `` without escaping `</`,
which lets a `` in skill output break out of the script element — is
the separate issue #1075.
- The xlsx preview (`XLSX.utils.sheet_to_html` → `innerHTML`) relies on SheetJS's
own cell-text escaping and is out of scope here.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.