GHSA-2hm2-hc3v-44h9

## Summary **Type:** Predictable identifier generation. The `toc` plugin and `TableOfContents` directive both default to generating heading IDs of the form `toc_1`, `toc_2`, `toc_3`, ... with no input-derived component. An attacker who can place a heading anywhere in the document can predict which `toc_N` ID it will receive, and can inject HTML elsewhere (in a non-heading context) that uses the same `id="toc_N"` to either (a) shadow the legitimate heading anchor, breaking same-page navigation, or (b) collide with CSS or JavaScript that targets `#toc_N` selectors, redirecting click handlers and styling to attacker-chosen content. **File:** `src/mistune/toc.py` line 36-37 (`heading_id = lambda token, index: "toc_" + str(index + 1)`); `src/mistune/directives/toc.py` line 33 (same default). **Root cause:** the default `heading_id` callback ignores the heading's text content and uses only the headings's order in the document. Two documents rendered together (or one document with attacker-influenced headings spliced into trusted content) produce overlapping `toc_N` IDs. Because `id` attribute uniqueness is required by HTML, browsers behaviour on duplicate IDs is undefined; `document.getElementById('toc_1')` returns the first match, `getElementsByTagName + querySelector` semantics differ across paths, and CSS rules targeting `#toc_1` apply to whichever element matches first in tree order. ## Affected Code **File:** `src/mistune/toc.py`, lines 33-39. ```python def add_toc_hook(md, min_level=1, max_level=3, heading_id=None): if heading_id is None: def heading_id(token, index): return "toc_" + str(index + 1) # <-- BUG: index-only ID, no slug derived from heading text ``` **File:** `src/mistune/directives/toc.py`, lines 32-33. ```python class TableOfContents(DirectivePlugin): def __init__(self, min_level=1, max_level=3): # ... def generate_heading_id(self, token, index): return "toc_" + str(index + 1) # <-- BUG: same predictable scheme ``` **Why it's wrong:** the standard markdown-engine convention (used by GitHub-flavoured Markdown, Sphinx, MkDocs, pandoc, every modern markdown renderer in production) is to slugify the heading TEXT for the ID — `<h1 id="introduction">Introduction</h1>` — with a numeric suffix appended only when slug collisions occur. mistune's default punts the slugification entirely and produces purely positional IDs that an attacker can predict in O(1). The downstream impacts: - Same-page links with `[click](#toc_1)` go to whichever element with `id="toc_1"` appears first in tree order. If the attacker can land any HTML element with `id="toc_1"` before the real heading (via inline_html with `escape=False`, via the include-directive HTML branch, via attacker-supplied content earlier in the document), navigation is hijacked. - CSS rules targeting `#toc_1` apply to the wrong element. - JavaScript bound to `document.getElementById('toc_1')` operates on the wrong element. - The TOC's own `<a href="#toc_1">` link in the rendered TOC list points to whichever element wins the duplicate-ID race. ## Exploit Chain 1. Application uses mistune with `add_toc_hook(md)` or `TableOfContents` directive enabled (the documented setup for sites with TOC support). 2. Application renders an attacker-supplied document, or splices attacker content into a trusted document. With `escape=False` (or via the include-directive `.html` branch covered by my prior advisory), the attacker can place `<a id="toc_1">...</a>` anywhere in the document. 3. mistune assigns `id="toc_1"` to the first heading. Now there are two elements with `id="toc_1"` in the page. 4. The rendered TOC contains `<a href="#toc_1">First heading</a>`. Clicking it navigates to whichever element with `id="toc_1"` appears first in tree order. If the attacker placed their `<a id="toc_1">` BEFORE the heading, navigation is hijacked. 5. Same-page CSS / JS / aria-described references to `#toc_1` similarly redirect. ## Security Impact **Severity:** sec-low. Not a direct XSS or RCE; the issue is identifier confusion that enables UI-redirection / navigation-hijack attacks. The realistic attacker capability is "make an internal anchor link go to attacker content instead of the real heading", or "make a CSS selector apply to attacker content", or "break aria/screen-reader associations". **Attacker capability:** with the ability to plant any HTML element with `id="toc_N"` in the document, hijack `<a href="#toc_N">` navigation and any CSS/JS targeting that ID. With `escape=False`, this is straightforward. With `escape=True`, the attacker needs another vector to land a raw `id` attribute (one of the include-directive branches, a sibling tooling pipeline that lets HTML through, etc.). **Preconditions:** application uses TOC + the default `heading_id` callback. If the application provides its own `heading_id` (e.g., one based on slugified heading text, with collision suffixes), this finding does not apply. **Differential:** PoC-verified against [email protected]: ```python import mistune from mistune.directives import RSTDirective, TableOfContents md = mistune.create_markdown(plugins=[RSTDirective([TableOfContents()])]) print(md(''' .. toc:: # Heading 1 # Heading 2 ''')) # Output (note: id="toc_1" / id="toc_2", purely positional): # <details class="toc" open> # <summary>Table of Contents</summary> # <ul> # <li><a href="#toc_1">Heading 1</a></li> # <li><a href="#toc_2">Heading 2</a></li> # </ul> # </details> # <h1 id="toc_1">Heading 1</h1> # <h1 id="toc_2">Heading 2</h1> ``` The patched build (with the suggested fix below) produces text-derived slugs like `id="heading-1"` and `id="heading-2"`, which are tied to content rather than position. ## Suggested Fix Default to slugifying the heading text: ```diff --- a/src/mistune/toc.py +++ b/src/mistune/toc.py @@ -33,9 +33,18 @@ def add_toc_hook(md, min_level=1, max_level=3, heading_id=None): if heading_id is None: + import re + _slug_re = re.compile(r"[^a-z0-9]+") + seen = {} def heading_id(token, index): - return "toc_" + str(index + 1) + text = striptags(md.renderer(md.inline(token["text"], {}), BlockState())) + slug = _slug_re.sub("-", text.lower()).strip("-") or "section" + n = seen.get(slug, 0) + seen[slug] = n + 1 + return slug if n == 0 else f"{slug}-{n}" ``` Same change applies to `src/mistune/directives/toc.py:33`. Existing applications that have hardcoded `#toc_N` anchors will break; document the migration in the changelog and consider providing an opt-out flag for the legacy behaviour. Add a regression test that asserts heading IDs are slug-derived, not position-derived, and that collisions get a `-N` suffix.