From 4249d117c0a677aaacf6bcb2d8da1d9d19c04157 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 14:18:45 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20FIX:=20escape=20user=20content?= =?UTF-8?q?=20in=20renderers=20and=20harden=20attrs=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the XSS advisories reported against default configurations in #143: - texmath: HTML-escape math content and equation-number labels in the default renderer (consistent with dollarmath/amsmath). Input such as `$$` previously emitted live markup. - dollarmath: escape equation labels in the generated `id` attribute and the permalink `href`. - myst_block: escape target labels `(name)=` in the rendered anchor. - attrs/attrs_block: when no `allowed` list is given, strip event-handler (`on*`) and `style` attributes by default (kept in meta `insecure_attrs`). Text spans `[text]{...}` now also honour the `allowed` list, which they previously bypassed. This is a baseline protection, not a full sanitiser. The admonition ReDoS (advisory 4) is addressed separately in #148. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DS3iVi5q1TepHaMymmmeJy --- CHANGELOG.md | 11 ++++++ mdit_py_plugins/attrs/index.py | 53 +++++++++++++++++++++++----- mdit_py_plugins/dollarmath/index.py | 4 +-- mdit_py_plugins/myst_blocks/index.py | 2 +- mdit_py_plugins/texmath/index.py | 7 ++-- tests/fixtures/myst_block.md | 2 +- tests/fixtures/texmath_bracket.md | 8 ++--- tests/fixtures/texmath_dollar.md | 8 ++--- tests/test_attrs.py | 40 +++++++++++++++++++++ tests/test_dollarmath.py | 19 ++++++++++ tests/test_myst_block.py | 8 +++++ tests/test_texmath.py | 16 +++++++++ 12 files changed, 155 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1b78b3..bd214af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Change Log +## Unreleased + +- 🔒 SECURITY: Escape user-controlled content in plugin renderers and harden the `attrs` default (#143) + + Addresses externally-reported XSS advisories affecting default configurations: + + - `texmath`: math content and equation-number labels are now HTML-escaped by the default renderer, matching `dollarmath` and `amsmath`. Previously input such as ``$$`` emitted live markup. + - `dollarmath`: equation labels are now escaped in the generated `id` attribute and permalink `href`. + - `myst_block`: target labels (``(name)=``) are now escaped in the rendered anchor. + - `attrs` / `attrs_block`: when no explicit `allowed` list is given, event-handler (`on*`) and `style` attributes are now stripped by default (removed entries are preserved in `token.meta["insecure_attrs"]`). Text spans (``[text]{...}``) now also honour the `allowed` list, which they previously bypassed entirely. This is a baseline protection, not a full sanitiser — pass an explicit `allowed` list (e.g. `("id", "class")`) to safely render untrusted input. + ## 0.7.0 - 2026-07-19 - ✨ NEW: Add section reference plugin (`section_ref`) (#144) diff --git a/mdit_py_plugins/attrs/index.py b/mdit_py_plugins/attrs/index.py index 0f608c8..1f62b3b 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -56,10 +56,19 @@ def attrs_plugin( :param allowed: A list of allowed attribute names. If not ``None``, any attributes not in this list will be removed and placed in the token's meta under the key "insecure_attrs". + If ``None`` (the default), event-handler (``on*``) and ``style`` + attributes are still removed as a baseline protection against script + and CSS injection. This is not a full sanitiser: to safely render + untrusted input, pass an explicit ``allowed`` list (e.g. + ``("id", "class")``). """ if spans: - md.inline.ruler.after(span_after, "span", _span_rule) + md.inline.ruler.after( + span_after, + "span", + partial(_span_rule, allowed=None if allowed is None else set(allowed)), + ) if after: md.inline.ruler.push( "attr", @@ -91,6 +100,11 @@ def attrs_block_plugin(md: MarkdownIt, *, allowed: Sequence[str] | None = None) :param allowed: A list of allowed attribute names. If not ``None``, any attributes not in this list will be removed and placed in the token's meta under the key "insecure_attrs". + If ``None`` (the default), event-handler (``on*``) and ``style`` + attributes are still removed as a baseline protection against script + and CSS injection. This is not a full sanitiser: to safely render + untrusted input, pass an explicit ``allowed`` list (e.g. + ``("id", "class")``). """ md.block.ruler.before("fence", "attr", _attr_block_rule) md.core.ruler.after( @@ -115,7 +129,9 @@ def _find_opening(tokens: Sequence[Token], index: int) -> int | None: return None -def _span_rule(state: StateInline, silent: bool) -> bool: +def _span_rule( + state: StateInline, silent: bool, *, allowed: set[str] | None = None +) -> bool: if state.src[state.pos] != "[": return False @@ -144,7 +160,7 @@ def _span_rule(state: StateInline, silent: bool) -> bool: state.pos = labelStart state.posMax = labelEnd token = state.push("span_open", "span", 1) - token.attrs = attrs # type: ignore[assignment] + _add_attrs(token, attrs, allowed) state.md.inline.tokenize(state) token = state.push("span_close", "span", -1) @@ -259,17 +275,38 @@ def _attr_resolve_block_rule(state: StateCore, *, allowed: set[str] | None) -> N len_tokens -= 1 +def _is_insecure_attr(key: str) -> bool: + """Return True for attributes that enable scripting or style injection. + + Event-handler attributes (``on*``, e.g. ``onclick``) execute JavaScript, and + ``style`` permits CSS-based injection, so both are stripped by default when + no explicit ``allowed`` list is given. + """ + key = key.lower() + return key == "style" or key.startswith("on") + + def _add_attrs( token: Token, attrs: dict[str, Any], allowed: set[str] | None, ) -> None: - """Add attributes to a token, skipping any disallowed attributes.""" - if allowed is not None and ( - disallowed := {k: v for k, v in attrs.items() if k not in allowed} - ): + """Add attributes to a token, skipping any disallowed attributes. + + When ``allowed`` is not ``None`` only those names are kept. When it is + ``None`` (the default) an insecure-by-default baseline still removes + event-handler (``on*``) and ``style`` attributes, so untrusted input cannot + inject script or CSS. Anything removed is preserved in + ``token.meta["insecure_attrs"]``. + """ + if allowed is not None: + disallowed = {k: v for k, v in attrs.items() if k not in allowed} + else: + disallowed = {k: v for k, v in attrs.items() if _is_insecure_attr(k)} + + if disallowed: token.meta["insecure_attrs"] = disallowed - attrs = {k: v for k, v in attrs.items() if k in allowed} + attrs = {k: v for k, v in attrs.items() if k not in disallowed} # attributes takes precedence over existing attributes token.attrs.update(attrs) diff --git a/mdit_py_plugins/dollarmath/index.py b/mdit_py_plugins/dollarmath/index.py index 57ba590..717e32f 100644 --- a/mdit_py_plugins/dollarmath/index.py +++ b/mdit_py_plugins/dollarmath/index.py @@ -79,7 +79,7 @@ def dollarmath_plugin( if label_renderer is None: _label_renderer = ( # noqa: E731 lambda label: ( - f'¶' + f'¶' ) ) else: @@ -123,7 +123,7 @@ def render_math_block_label( env: EnvType, ) -> str: content = _renderer(str(tokens[idx].content).strip(), {"display_mode": True}) - _id = tokens[idx].info + _id = escapeHtml(tokens[idx].info) label = _label_renderer(tokens[idx].info) return f'
\n{label}\n{content}\n
\n' diff --git a/mdit_py_plugins/myst_blocks/index.py b/mdit_py_plugins/myst_blocks/index.py index 11fc0fb..14fbbd3 100644 --- a/mdit_py_plugins/myst_blocks/index.py +++ b/mdit_py_plugins/myst_blocks/index.py @@ -152,7 +152,7 @@ def render_myst_target( options: OptionsDict, env: EnvType, ) -> str: - label = tokens[idx].content + label = escapeHtml(tokens[idx].content) class_name = "myst-target" target = f'({label})=' return f'
{target}
' diff --git a/mdit_py_plugins/texmath/index.py b/mdit_py_plugins/texmath/index.py index 67372fa..19e8714 100644 --- a/mdit_py_plugins/texmath/index.py +++ b/mdit_py_plugins/texmath/index.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING, Any, TypedDict from markdown_it import MarkdownIt -from markdown_it.common.utils import charCodeAt +from markdown_it.common.utils import charCodeAt, escapeHtml if TYPE_CHECKING: from markdown_it.renderer import RendererProtocol @@ -47,7 +47,7 @@ def render_math_inline( env: EnvType, ) -> str: return rule_inline["tmpl"].format( # noqa: B023 - render(tokens[idx].content, False, macros) + escapeHtml(render(tokens[idx].content, False, macros)) ) md.add_render_rule(rule_inline["name"], render_math_inline) @@ -65,7 +65,8 @@ def render_math_block( env: EnvType, ) -> str: return rule_block["tmpl"].format( # noqa: B023 - render(tokens[idx].content, True, macros), tokens[idx].info + escapeHtml(render(tokens[idx].content, True, macros)), + escapeHtml(tokens[idx].info), ) md.add_render_rule(rule_block["name"], render_math_block) diff --git a/tests/fixtures/myst_block.md b/tests/fixtures/myst_block.md index 1ec05e3..515daf4 100644 --- a/tests/fixtures/myst_block.md +++ b/tests/fixtures/myst_block.md @@ -52,7 +52,7 @@ Target characters: . (a bc |@<>*./_-+:)= . -
(a bc |@<>*./_-+:)=
+
(a bc |@<>*./_-+:)=
. Empty target: diff --git a/tests/fixtures/texmath_bracket.md b/tests/fixtures/texmath_bracket.md index 2f00e07..07000f1 100644 --- a/tests/fixtures/texmath_bracket.md +++ b/tests/fixtures/texmath_bracket.md @@ -23,7 +23,7 @@ simple equation including special html character. (valid=True) . \(1+1<3\) . -

1+1<3

+

1+1<3

. equation including backslashes. (valid=True) @@ -239,8 +239,8 @@ multiline equation. (valid=True) .
\\begin{matrix} - f & = & 2 + x + 3 \\ - & = & 5 + x + f & = & 2 + x + 3 \\ + & = & 5 + x \\end{matrix}
. @@ -253,7 +253,7 @@ vector equation. (valid=True) .
\\begin{pmatrix}x_2 \\\\ y_2 \\end{pmatrix} = -\\begin{pmatrix} A & B \\\\ C & D \\end{pmatrix}\\cdot +\\begin{pmatrix} A & B \\\\ C & D \\end{pmatrix}\\cdot \\begin{pmatrix} x_1 \\\\ y_1 \\end{pmatrix}
. diff --git a/tests/fixtures/texmath_dollar.md b/tests/fixtures/texmath_dollar.md index 26357b3..9f909cb 100644 --- a/tests/fixtures/texmath_dollar.md +++ b/tests/fixtures/texmath_dollar.md @@ -23,7 +23,7 @@ simple equation including special html character. (valid=True) . $1+1<3$ . -

1+1<3

+

1+1<3

. equation including backslashes. (valid=True) @@ -239,8 +239,8 @@ $$\\begin{matrix} .
\\begin{matrix} - f & = & 2 + x + 3 \\ - & = & 5 + x + f & = & 2 + x + 3 \\ + & = & 5 + x \\end{matrix}
. @@ -253,7 +253,7 @@ $$\\begin{pmatrix}x_2 \\\\ y_2 \\end{pmatrix} = .
\\begin{pmatrix}x_2 \\\\ y_2 \\end{pmatrix} = -\\begin{pmatrix} A & B \\\\ C & D \\end{pmatrix}\\cdot +\\begin{pmatrix} A & B \\\\ C & D \\end{pmatrix}\\cdot \\begin{pmatrix} x_1 \\\\ y_1 \\end{pmatrix}
. diff --git a/tests/test_attrs.py b/tests/test_attrs.py index cec33d1..c792071 100644 --- a/tests/test_attrs.py +++ b/tests/test_attrs.py @@ -37,3 +37,43 @@ def test_attrs_allowed(data_regression): `inline`{safe=a danger=b} """) data_regression.check([t.as_dict() for t in tokens]) + + +@pytest.mark.parametrize( + "src,leaked", + [ + ('[x]{onclick="alert(1)"}', "onclick"), + ('[x]{OnClick="alert(1)"}', "onclick"), # case-insensitive + ('[x]{style="color:red"}', "style"), + ('![a](i.png){onerror="alert(1)"}', "onerror"), + ], +) +def test_event_handler_and_style_stripped_by_default(src, leaked): + """With no ``allowed`` list, on* and style attributes are removed (XSS).""" + md = MarkdownIt("commonmark").use(attrs_plugin, spans=True).use(attrs_block_plugin) + text = md.render(src) + assert leaked.lower() not in text.lower() + + +def test_block_event_handler_stripped_by_default(): + md = MarkdownIt("commonmark").use(attrs_plugin).use(attrs_block_plugin) + text = md.render('{onmouseover="alert(1)"}\nparagraph\n') + assert "onmouseover" not in text + + +def test_benign_attrs_preserved_by_default(): + """The insecure-by-default baseline only strips on*/style, not benign attrs.""" + md = MarkdownIt("commonmark").use(attrs_plugin, spans=True) + text = md.render("[x]{#a .b width=100 data-y=z}") + assert 'id="a"' in text + assert 'width="100"' in text + assert 'data-y="z"' in text + + +def test_span_respects_allowed_list(): + """Spans now honour an explicit allow-list (previously bypassed entirely).""" + md = MarkdownIt("commonmark").use(attrs_plugin, spans=True, allowed=["id", "class"]) + text = md.render("[x]{#a .b width=100 onclick=e}") + assert 'id="a"' in text + assert "width" not in text + assert "onclick" not in text diff --git a/tests/test_dollarmath.py b/tests/test_dollarmath.py index a43dc10..55251e9 100644 --- a/tests/test_dollarmath.py +++ b/tests/test_dollarmath.py @@ -104,3 +104,22 @@ def test_dollarmath_fixtures(line, title, input, expected): text = md.render(input) print(text) assert text.rstrip() == expected.rstrip() + + +def test_label_is_html_escaped(): + """Equation labels must be HTML-escaped in the id attribute and href (XSS).""" + md = MarkdownIt("commonmark").use(dollarmath_plugin) + text = md.render('$$a=1$$ (">)') + # the payload must not break out of the attribute / inject an element + assert ")') + assert ")=") + assert "