anthropics / anthropics/skills

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

Ouverte Adaptée aux débutants
#1,394 2 commentaires 2 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Python
Étoiles
176k
Forks
20.9k
Merge moyen
7 h 21 min
PR mergées (30 j)
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.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Piste de recherche

Start in skills/skill-creator/eval-viewer/viewer.html, especially escapeHtml(), renderGrades, and renderBenchmark. Reproduce by putting the given quoted evidence string in a grading result, opening the Benchmark tab, and hovering the assertion badge. Done means attribute-breaking evidence and the listed metadata/config/delta fields render as text, with normal data visually unchanged.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
html, javascript
Domaine
frontend, security
Type d'issue
Bug
Difficulté
2/5
Temps estimé
1-3 heures
Activité
Active
Clarté
Clairement spécifiée
Accessibilité débutants
84/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.