anthropics / anthropics/skills

skill-creator: eval-viewer escapeHtml is not attribute-safe and is applied inconsistently (display-path XSS)

Open
#1,394 2 comments 2 reactions 0 assignees View on GitHub
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.