From 046fc1629bb70e40b59db4065cbcad2c76fbb137 Mon Sep 17 00:00:00 2001 From: Kyle King Date: Tue, 21 Jul 2026 09:02:19 -0600 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20FIX:=20escape=20default=20re?= =?UTF-8?q?nderers=20and=20harden=20attrs=20against=20XSS=20(#143)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the XSS advisories from #143 where plugins interpolated attacker-controlled markdown into their default HTML render rules without escaping: - texmath: escape the math source in the default render() stub and the math_block_eqno equation number, across every delimiter flavor - dollarmath: escape the equation label in the id/href attributes (the content was already escaped) - myst_block: escape the target label in the body and href Harden attrs/attrs_block: with no explicit allowed list, divert event-handler (on*) and style attributes to token.meta["insecure_attrs"] instead of emitting them, so the default configuration cannot inject script. The inline span path previously set token.attrs directly and bypassed the filter; route it through _add_attrs with the allowed set threaded in. Other attribute names are unchanged. Update the texmath/myst fixtures that encoded the old unescaped output, and add a categorical guard (tests/test_xss.py) that renders injection probes through each plugin to catch the next unescaped interpolation. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01L4mwfRr7ZkjiwPATm6Q4ue --- CHANGELOG.md | 30 ++++++++ mdit_py_plugins/attrs/index.py | 44 +++++++++-- mdit_py_plugins/dollarmath/index.py | 4 +- mdit_py_plugins/myst_blocks/index.py | 2 +- mdit_py_plugins/texmath/index.py | 10 ++- tests/fixtures/myst_block.md | 2 +- tests/fixtures/texmath_bracket.md | 8 +- tests/fixtures/texmath_dollar.md | 8 +- tests/test_xss.py | 108 +++++++++++++++++++++++++++ 9 files changed, 193 insertions(+), 23 deletions(-) create mode 100644 tests/test_xss.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a1b78b3..19cb064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,35 @@ # Change Log +## Unreleased + +- πŸ› FIX: Escape attacker-controlled content in default HTML renderers to prevent XSS (#143) + + Several plugins interpolated captured markdown into their default HTML render + rules without escaping, so untrusted input could inject live markup even with + raw HTML disabled: + + - `texmath` emitted the math source (and the `math_block_eqno` equation number) + verbatim into ``/`` templates across every delimiter flavor + - `dollarmath` emitted the equation label into `id=`/`href=` attributes (the + equation content was already escaped) + - `myst_block` emitted the target label into an `` body and `href` + + All three now escape via `markdown_it.common.utils.escapeHtml`. Downstream + integrators that register their own render rules (e.g. `myst-parser`) are + unaffected. + +- πŸ”’ SECURITY: `attrs`/`attrs_block` no longer emit event-handler or `style` attributes by default (#143) + + With no explicit `allowed` list, `on*` and `style` attributes are now diverted + to `token.meta["insecure_attrs"]` (matching the existing allow-list behavior) + instead of reaching the output, so `[x]{onclick="..."}` can no longer inject + script under the default configuration. Other attribute names (`id`, `class`, + `data-*`, etc.) are unchanged; pass an explicit `allowed` list to restrict + further. + + A categorical guard (`tests/test_xss.py`) now renders injection probes through + each plugin to catch the next unescaped interpolation. + ## 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..45777d5 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -56,10 +56,18 @@ 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". + When ``None`` (the default), event-handler (``on*``) and ``style`` + attributes are still diverted to "insecure_attrs", since they inject + script even when raw HTML is disabled; pass an explicit allow-list to + restrict attributes further for untrusted input. """ 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 +99,10 @@ 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". + When ``None`` (the default), event-handler (``on*``) and ``style`` + attributes are still diverted to "insecure_attrs", since they inject + script even when raw HTML is disabled; pass an explicit allow-list to + restrict attributes further for untrusted input. """ md.block.ruler.before("fence", "attr", _attr_block_rule) md.core.ruler.after( @@ -115,7 +127,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 +158,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 +273,31 @@ def _attr_resolve_block_rule(state: StateCore, *, allowed: set[str] | None) -> N len_tokens -= 1 +def _is_event_or_style(key: str) -> bool: + """Attributes that inject script even when raw HTML is disabled.""" + lowered = key.lower() + return lowered.startswith("on") or lowered == "style" + + 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, diverting disallowed ones to meta. + + With an explicit ``allowed`` set, anything outside it is diverted. Without one + (the default), event-handler (``on*``) and ``style`` attributes are still + diverted, so the default configuration cannot emit a script vector. + """ + if allowed is None: + disallowed = {k: v for k, v in attrs.items() if _is_event_or_style(k)} + else: + disallowed = {k: v for k, v in attrs.items() if k not in allowed} + + 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..9ff9978 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 @@ -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 + render(tokens[idx].content, True, macros), + escapeHtml(tokens[idx].info), ) md.add_render_rule(rule_block["name"], render_math_block) @@ -169,7 +170,10 @@ def dollar_post(src: str, end: int) -> bool: def render(tex: str, displayMode: bool, macros: Any) -> str: - return tex + # The captured math source is attacker-controlled and interpolated into HTML + # templates, so escape it; a real (KaTeX-style) renderer would emit safe markup + # of its own and replace this stub. + return escapeHtml(tex) # TODO better HTML renderer port for math # try: # res = katex.renderToString(tex,{throwOnError:False,displayMode,macros}) 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_xss.py b/tests/test_xss.py new file mode 100644 index 0000000..603ee6c --- /dev/null +++ b/tests/test_xss.py @@ -0,0 +1,108 @@ +"""Categorical XSS guard: render injection probes through each plugin's defaults. + +Several plugins register default HTML render rules that interpolate +attacker-controlled token content into markup. Rather than trust review to catch +the next unescaped interpolation, this drives each plugin end-to-end with a probe +and asserts the raw marker never reaches the output (``markdown-it`` runs with raw +HTML disabled, so a correctly-escaped plugin emits ``<xss>`` instead). + +Two probe shapes cover the two sinks: + +- ``">`` for HTML/attribute contexts: a plugin that escapes turns the ``<`` + into ``<`` so the ``)", + " None: + md = MarkdownIt("commonmark") + case.configure(md) + out = md.render(case.payload) + assert case.marker not in out, ( + f"{case.id} emitted {case.marker!r} unescaped: {out!r}" + ) From 811e7e062f1a9afba2939869c400f449d6ecfd31 Mon Sep 17 00:00:00 2001 From: Kyle King Date: Tue, 21 Jul 2026 09:47:26 -0600 Subject: [PATCH 2/4] test(143): add categorical XSS guard across plugin renderers --- CHANGELOG.md | 2 + tests/test_xss.py | 108 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/test_xss.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bd214af..2d24e8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ - `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. + A categorical guard (`tests/test_xss.py`) renders injection probes through each plugin's default renderer, so a future unescaped interpolation is caught before review. + ## 0.7.0 - 2026-07-19 - ✨ NEW: Add section reference plugin (`section_ref`) (#144) diff --git a/tests/test_xss.py b/tests/test_xss.py new file mode 100644 index 0000000..603ee6c --- /dev/null +++ b/tests/test_xss.py @@ -0,0 +1,108 @@ +"""Categorical XSS guard: render injection probes through each plugin's defaults. + +Several plugins register default HTML render rules that interpolate +attacker-controlled token content into markup. Rather than trust review to catch +the next unescaped interpolation, this drives each plugin end-to-end with a probe +and asserts the raw marker never reaches the output (``markdown-it`` runs with raw +HTML disabled, so a correctly-escaped plugin emits ``<xss>`` instead). + +Two probe shapes cover the two sinks: + +- ``">`` for HTML/attribute contexts: a plugin that escapes turns the ``<`` + into ``<`` so the ``)", + " None: + md = MarkdownIt("commonmark") + case.configure(md) + out = md.render(case.payload) + assert case.marker not in out, ( + f"{case.id} emitted {case.marker!r} unescaped: {out!r}" + ) From e4eb4cdd66be495bc9404c9582b9c880654631a4 Mon Sep 17 00:00:00 2001 From: Kyle King Date: Tue, 21 Jul 2026 10:54:42 -0600 Subject: [PATCH 3/4] refactor: minimize diff --- mdit_py_plugins/attrs/index.py | 20 +++++++++++++------- mdit_py_plugins/texmath/index.py | 3 --- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/mdit_py_plugins/attrs/index.py b/mdit_py_plugins/attrs/index.py index 31b1c47..6db8ed6 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -273,10 +273,15 @@ def _attr_resolve_block_rule(state: StateCore, *, allowed: set[str] | None) -> N len_tokens -= 1 -def _is_event_or_style(key: str) -> bool: - """Attributes that inject script even when raw HTML is disabled.""" - lowered = key.lower() - return lowered.startswith("on") or lowered == "style" +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( @@ -290,10 +295,11 @@ def _add_attrs( (the default), event-handler (``on*``) and ``style`` attributes are still diverted, so the default configuration cannot emit a script vector. """ - if allowed is None: - disallowed = {k: v for k, v in attrs.items() if _is_event_or_style(k)} - else: + 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 not in disallowed} diff --git a/mdit_py_plugins/texmath/index.py b/mdit_py_plugins/texmath/index.py index 9ff9978..efbc451 100644 --- a/mdit_py_plugins/texmath/index.py +++ b/mdit_py_plugins/texmath/index.py @@ -170,9 +170,6 @@ def dollar_post(src: str, end: int) -> bool: def render(tex: str, displayMode: bool, macros: Any) -> str: - # The captured math source is attacker-controlled and interpolated into HTML - # templates, so escape it; a real (KaTeX-style) renderer would emit safe markup - # of its own and replace this stub. return escapeHtml(tex) # TODO better HTML renderer port for math # try: From c977b60f1db2818a3b7ac7d6b7a2f27ea825d9cd Mon Sep 17 00:00:00 2001 From: Kyle King Date: Tue, 21 Jul 2026 10:56:18 -0600 Subject: [PATCH 4/4] docs: also rollback cosmetic docstring changes to minimize diff --- mdit_py_plugins/attrs/index.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/mdit_py_plugins/attrs/index.py b/mdit_py_plugins/attrs/index.py index 6db8ed6..1f62b3b 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -56,10 +56,11 @@ 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". - When ``None`` (the default), event-handler (``on*``) and ``style`` - attributes are still diverted to "insecure_attrs", since they inject - script even when raw HTML is disabled; pass an explicit allow-list to - restrict attributes further for untrusted input. + 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: @@ -99,10 +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". - When ``None`` (the default), event-handler (``on*``) and ``style`` - attributes are still diverted to "insecure_attrs", since they inject - script even when raw HTML is disabled; pass an explicit allow-list to - restrict attributes further for untrusted input. + 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( @@ -289,11 +291,13 @@ def _add_attrs( attrs: dict[str, Any], allowed: set[str] | None, ) -> None: - """Add attributes to a token, diverting disallowed ones to meta. + """Add attributes to a token, skipping any disallowed attributes. - With an explicit ``allowed`` set, anything outside it is diverted. Without one - (the default), event-handler (``on*``) and ``style`` attributes are still - diverted, so the default configuration cannot emit a script vector. + 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}