From 4249d117c0a677aaacf6bcb2d8da1d9d19c04157 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 14:18:45 +0000 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=94=92=20FIX:=20escape=20user=20con?= =?UTF-8?q?tent=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 " Date: Tue, 21 Jul 2026 09:02:19 -0600 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=90=9B=20FIX:=20escape=20default=20?= =?UTF-8?q?renderers=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 03/11] 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 04/11] 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 05/11] 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} From 4e50b7826b7275ad95806c7fdc55d595c6d7bfb3 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 9 Nov 2026 20:24:27 +0100 Subject: [PATCH 06/11] =?UTF-8?q?=F0=9F=94=A7=20MAINTAIN:=20drop=20changel?= =?UTF-8?q?og=20entry=20(written=20at=20release)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d24e8a..a1b78b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,5 @@ # 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. - - 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) From da214c111d00266e4c9a5b43d1457a33da2e8c8c Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 9 Nov 2026 20:26:12 +0100 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=90=9B=20FIX:=20attrs=20keeps=20`st?= =?UTF-8?q?yle`=20by=20default,=20strips=20only=20`on*`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `style` cannot run script, and MyST-Parser reads `{style=lower-alpha}` on an ordered list to choose its numbering, so stripping it by default broke existing documents. The default baseline now removes only event-handler (`on*`) attributes. --- mdit_py_plugins/attrs/index.py | 41 +++++++++++++++-------------- tests/test_attrs.py | 47 +++++++++++++++++++++++++++++++--- tests/test_xss.py | 6 ----- 3 files changed, 63 insertions(+), 31 deletions(-) diff --git a/mdit_py_plugins/attrs/index.py b/mdit_py_plugins/attrs/index.py index 1f62b3b..510d8a7 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -56,11 +56,12 @@ 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 ``None`` (the default), event-handler attributes + (any name starting with ``on``, such as ``onclick``) are still removed, + as a baseline protection against script injection. + Other attributes, including ``style``, are kept. + This is not a full sanitiser: to safely render untrusted input, + pass an explicit ``allowed`` list (e.g. ``("id", "class")``). """ if spans: @@ -100,11 +101,12 @@ 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")``). + If ``None`` (the default), event-handler attributes + (any name starting with ``on``, such as ``onclick``) are still removed, + as a baseline protection against script injection. + Other attributes, including ``style``, are kept. + 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( @@ -276,14 +278,12 @@ def _attr_resolve_block_rule(state: StateCore, *, allowed: set[str] | None) -> N def _is_insecure_attr(key: str) -> bool: - """Return True for attributes that enable scripting or style injection. + """Return True for event-handler attributes (``on*``, e.g. ``onclick``). - 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. + They execute JavaScript, so they are removed by default + when no explicit ``allowed`` list is given. """ - key = key.lower() - return key == "style" or key.startswith("on") + return key.lower().startswith("on") def _add_attrs( @@ -293,11 +293,10 @@ def _add_attrs( ) -> None: """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"]``. + When ``allowed`` is not ``None`` only those names are kept. + When it is ``None`` (the default) event-handler (``on*``) attributes + are still removed, so untrusted input cannot inject script. + 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} diff --git a/tests/test_attrs.py b/tests/test_attrs.py index c792071..1a8501c 100644 --- a/tests/test_attrs.py +++ b/tests/test_attrs.py @@ -44,12 +44,11 @@ def test_attrs_allowed(data_regression): [ ('[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).""" +def test_event_handler_stripped_by_default(src, leaked): + """With no ``allowed`` list, on* 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() @@ -61,8 +60,48 @@ def test_block_event_handler_stripped_by_default(): assert "onmouseover" not in text +def test_event_handler_moved_to_meta(): + """A removed on* attribute is kept in ``token.meta["insecure_attrs"]``.""" + md = MarkdownIt("commonmark").use(attrs_block_plugin) + tokens = md.parse("{#a onclick=x}\nparagraph\n") + assert tokens[0].type == "paragraph_open" + assert tokens[0].attrs == {"id": "a"} + assert tokens[0].meta == {"insecure_attrs": {"onclick": "x"}} + + +@pytest.mark.parametrize( + "src,token_type,expected", + [ + pytest.param( + "{style=lower-alpha}\n1. a\n", + "ordered_list_open", + "lower-alpha", + id="block-list", + ), + pytest.param('[x]{style="color:red"}', "span_open", "color:red", id="span"), + pytest.param( + '![a](i.png){style="width:1em"}', "image", "width:1em", id="image" + ), + ], +) +def test_style_kept_by_default(src, token_type, expected): + """With no ``allowed`` list, ``style`` is kept (it does not run script). + + MyST-Parser reads ``{style=lower-alpha}`` on an ordered list + to choose its numbering, so dropping it would break documents. + """ + md = MarkdownIt("commonmark").use(attrs_plugin, spans=True).use(attrs_block_plugin) + tokens = md.parse(src) + found = [t for t in tokens if t.type == token_type] + found += [c for t in tokens for c in (t.children or []) if c.type == token_type] + assert len(found) == 1 + assert found[0].attrs.get("style") == expected + assert "insecure_attrs" not in found[0].meta + assert f'style="{expected}"' in md.render(src) + + def test_benign_attrs_preserved_by_default(): - """The insecure-by-default baseline only strips on*/style, not benign attrs.""" + """The insecure-by-default baseline only strips on*, 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 diff --git a/tests/test_xss.py b/tests/test_xss.py index 603ee6c..3c5c7f7 100644 --- a/tests/test_xss.py +++ b/tests/test_xss.py @@ -83,12 +83,6 @@ class Case: '![a](img){onxss="alert(1)"}\n', "onxss=", ), - Case( - "attrs-span-style", - lambda md: md.use(attrs_plugin, spans=True), - '[click]{style="color:red"}\n', - "style=", - ), Case( "attrs-block-event", lambda md: md.use(attrs_block_plugin), From f5ef22aa28224a53ee2bd991d2eff01f192bc200 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 9 Nov 2026 20:27:28 +0100 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=94=92=20FIX:=20attrs=20validates?= =?UTF-8?q?=20URL-valued=20attributes=20with=20`md.validateLink`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[a](https://x){href="javascript:alert(1)"}` rendered a live `javascript:` link even with `html=False`, bypassing the check markdown-it applies to `[a](javascript:...)` itself. `href`, `src`, `action`, `formaction` and `xlink:href` (any case) are now removed, into `token.meta["insecure_attrs"]`, when `md.validateLink` rejects their normalised value, whatever `allowed` says. Tabs, newlines and surrounding control characters, which browsers ignore when reading the scheme, are removed before the check. --- mdit_py_plugins/attrs/index.py | 43 ++++++++++- tests/test_attrs.py | 133 +++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 3 deletions(-) diff --git a/mdit_py_plugins/attrs/index.py b/mdit_py_plugins/attrs/index.py index 510d8a7..f3c8b8c 100644 --- a/mdit_py_plugins/attrs/index.py +++ b/mdit_py_plugins/attrs/index.py @@ -2,6 +2,7 @@ from collections.abc import Sequence from functools import partial +import re from typing import Any from markdown_it import MarkdownIt @@ -60,6 +61,10 @@ def attrs_plugin( (any name starting with ``on``, such as ``onclick``) are still removed, as a baseline protection against script injection. Other attributes, including ``style``, are kept. + Whatever ``allowed`` says, the URL-valued attributes + ``href``, ``src``, ``action``, ``formaction`` and ``xlink:href`` + are removed when ``md.validateLink`` rejects their value + (for example a ``javascript:`` URL), as for Markdown links and images. This is not a full sanitiser: to safely render untrusted input, pass an explicit ``allowed`` list (e.g. ``("id", "class")``). """ @@ -105,6 +110,10 @@ def attrs_block_plugin(md: MarkdownIt, *, allowed: Sequence[str] | None = None) (any name starting with ``on``, such as ``onclick``) are still removed, as a baseline protection against script injection. Other attributes, including ``style``, are kept. + Whatever ``allowed`` says, the URL-valued attributes + ``href``, ``src``, ``action``, ``formaction`` and ``xlink:href`` + are removed when ``md.validateLink`` rejects their value + (for example a ``javascript:`` URL), as for Markdown links and images. This is not a full sanitiser: to safely render untrusted input, pass an explicit ``allowed`` list (e.g. ``("id", "class")``). """ @@ -162,7 +171,7 @@ def _span_rule( state.pos = labelStart state.posMax = labelEnd token = state.push("span_open", "span", 1) - _add_attrs(token, attrs, allowed) + _add_attrs(state.md, token, attrs, allowed) state.md.inline.tokenize(state) token = state.push("span_close", "span", -1) @@ -195,7 +204,7 @@ def _attr_inline_rule( attr_token = state.tokens[token_index] if "class" in attrs and "class" in token.attrs: attrs["class"] = f"{token.attrs['class']} {attrs['class']}" - _add_attrs(attr_token, attrs, allowed) + _add_attrs(state.md, attr_token, attrs, allowed) return True @@ -271,12 +280,19 @@ def _attr_resolve_block_rule(state: StateCore, *, allowed: set[str] | None) -> N if key == "class" or key not in next_token.attrs: next_token.attrs[key] = value else: - _add_attrs(next_token, state.tokens[i].attrs, allowed) + _add_attrs(state.md, next_token, state.tokens[i].attrs, allowed) state.tokens.pop(i) len_tokens -= 1 +_URL_ATTRS = frozenset(("href", "src", "action", "formaction", "xlink:href")) +"""Attributes whose value is a URL, checked with ``md.validateLink``.""" + +_URL_IGNORED_RE = re.compile(r"[\t\n\r]") +_URL_STRIP = "".join(chr(i) for i in range(0x21)) + + def _is_insecure_attr(key: str) -> bool: """Return True for event-handler attributes (``on*``, e.g. ``onclick``). @@ -286,7 +302,21 @@ def _is_insecure_attr(key: str) -> bool: return key.lower().startswith("on") +def _is_valid_url(md: MarkdownIt, value: Any) -> bool: + """Return True if ``md.validateLink`` accepts a URL-valued attribute. + + Browsers ignore ASCII tabs and newlines anywhere in a URL, + and control characters and spaces around it, before reading its scheme + (so ``javascript:`` is a ``javascript:`` URL). + The value is emitted as written, so those characters are removed + before it is normalised and validated. + """ + url = _URL_IGNORED_RE.sub("", str(value)).strip(_URL_STRIP) + return md.validateLink(md.normalizeLink(url)) + + def _add_attrs( + md: MarkdownIt, token: Token, attrs: dict[str, Any], allowed: set[str] | None, @@ -296,12 +326,19 @@ def _add_attrs( When ``allowed`` is not ``None`` only those names are kept. When it is ``None`` (the default) event-handler (``on*``) attributes are still removed, so untrusted input cannot inject script. + In both cases, a URL-valued attribute (``href``, ``src``, ...) + is removed if ``md.validateLink`` rejects it. 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)} + disallowed.update( + (k, v) + for k, v in attrs.items() + if k not in disallowed and k.lower() in _URL_ATTRS and not _is_valid_url(md, v) + ) if disallowed: token.meta["insecure_attrs"] = disallowed diff --git a/tests/test_attrs.py b/tests/test_attrs.py index 1a8501c..8a0411c 100644 --- a/tests/test_attrs.py +++ b/tests/test_attrs.py @@ -116,3 +116,136 @@ def test_span_respects_allowed_list(): assert 'id="a"' in text assert "width" not in text assert "onclick" not in text + + +@pytest.mark.parametrize( + "src,expected,removed", + [ + pytest.param( + '[a](https://x){href="javascript:alert(1)"}', + '

a

\n', + {"href": "javascript:alert(1)"}, + id="link-href", + ), + pytest.param( + '[a](https://x){HREF="JavaScript:alert(1)"}', + '

a

\n', + {"HREF": "JavaScript:alert(1)"}, + id="link-href-case", + ), + pytest.param( + '[a](https://x){href="java\tscript:alert(1)"}', + '

a

\n', + {"href": "java\tscript:alert(1)"}, + id="link-href-tab", + ), + pytest.param( + '[a](https://x){href="\x01 javascript:alert(1)"}', + '

a

\n', + {"href": "\x01 javascript:alert(1)"}, + id="link-href-leading-control", + ), + pytest.param( + '![i](x.png){src="javascript:alert(1)"}', + '

i

\n', + {"src": "javascript:alert(1)"}, + id="image-src", + ), + pytest.param( + '[x]{href="vbscript:x"}', + "

x

\n", + {"href": "vbscript:x"}, + id="span-href", + ), + pytest.param( + '{formaction="javascript:x"}\npara\n', + "

para

\n", + {"formaction": "javascript:x"}, + id="block-formaction", + ), + pytest.param( + '{action="data:text/html,x"}\npara\n', + "

para

\n", + {"action": "data:text/html,x"}, + id="block-action", + ), + pytest.param( + '{xlink:href="javascript:x"}\npara\n', + "

para

\n", + {"xlink:href": "javascript:x"}, + id="block-xlink-href", + ), + ], +) +def test_url_attrs_validated(src, expected, removed): + """URL-valued attributes go through ``md.validateLink``, like link destinations. + + Otherwise ``{href="javascript:..."}`` would bypass the check markdown-it + applies to ``[a](javascript:...)``, even with ``html=False``. + """ + md = ( + MarkdownIt("commonmark", {"html": False}) + .use(attrs_plugin, spans=True) + .use(attrs_block_plugin) + ) + assert md.render(src) == expected + tokens = md.parse(src) + metas = [t.meta for t in tokens] + [ + c.meta for t in tokens for c in (t.children or []) + ] + assert {"insecure_attrs": removed} in metas + + +@pytest.mark.parametrize( + "src,expected", + [ + pytest.param( + '[a](https://x){href="https://ok"}', + '

a

\n', + id="link-https", + ), + pytest.param( + '[a](https://x){href="#frag"}', + '

a

\n', + id="link-fragment", + ), + pytest.param( + '![i](x.png){src="data:image/png;base64,AAA"}', + '

i

\n', + id="image-data-png", + ), + pytest.param( + '{href="https://ok"}\npara\n', + '

para

\n', + id="block-https", + ), + ], +) +def test_url_attrs_valid_kept(src, expected): + """URLs that ``md.validateLink`` accepts are kept as written.""" + md = ( + MarkdownIt("commonmark", {"html": False}) + .use(attrs_plugin, spans=True) + .use(attrs_block_plugin) + ) + assert md.render(src) == expected + + +def test_url_attrs_validated_when_allowed(): + """An explicit ``allowed`` list does not skip the URL check.""" + md = MarkdownIt("commonmark", {"html": False}).use(attrs_plugin, allowed=["href"]) + src = '[a](https://x){href="javascript:alert(1)"}' + assert md.render(src) == '

a

\n' + link_open = md.parse(src)[1].children[0] + assert link_open.meta == {"insecure_attrs": {"href": "javascript:alert(1)"}} + + +def test_url_attrs_use_custom_validate_link(): + """A ``validateLink`` override on the instance is honoured.""" + md = MarkdownIt("commonmark", {"html": False}).use(attrs_block_plugin) + md.validateLink = lambda url: not url.startswith("https://blocked") # type: ignore[method-assign] + assert md.render('{href="https://blocked/x"}\npara\n') == "

para

\n" + assert ( + md.render('{href="https://fine/x"}\npara\n') + == '

para

\n' + ) From 13912e333d0b1fa333a0ab2b775aac4e2e20d7d6 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 9 Nov 2026 20:28:04 +0100 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=A7=AA=20TEST:=20make=20the=20dolla?= =?UTF-8?q?rmath=20label=20XSS=20tests=20reach=20the=20label=20renderer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The payload `(">)` contains `(1)`, which ends the label parse, so the input rendered as a paragraph and both tests passed without the fix. The new payload has no parentheses; the test asserts it parses as `math_block_label` and checks the exact escaped output, for the default and an identity `label_normalizer`, with `html=False`. --- tests/test_dollarmath.py | 46 ++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/tests/test_dollarmath.py b/tests/test_dollarmath.py index 55251e9..8a38ec2 100644 --- a/tests/test_dollarmath.py +++ b/tests/test_dollarmath.py @@ -106,20 +106,34 @@ def test_dollarmath_fixtures(line, title, input, expected): 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 "\n' + f'' + "ΒΆ\na=1\n\n" ) - text = md.render('$$a=1$$ (">)') - assert " Date: Mon, 9 Nov 2026 20:29:31 +0100 Subject: [PATCH 10/11] =?UTF-8?q?=F0=9F=A7=AA=20TEST:=20XSS=20guard=20with?= =?UTF-8?q?=20`html=3DFalse`,=20reach=20checks=20and=20more=20sinks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Build markdown-it with `html=False`: the `commonmark` preset allows raw HTML, so the docstring's claim that it is disabled was wrong. - Each case states text the output must contain, so a payload the plugin does not parse cannot pass without testing anything. - Add quote-only attribute breakout probes (`a" onxss="1`) for the dollarmath label, the myst target and the colon_fence class, which the `">` probe alone does not catch. - Add colon_fence block name and tasklists `label_after` cases (#165), and a `javascript:` URL in an attrs `href`. - The texmath equation number case passed without its fix, because texmath never renders equation numbers (its render rules close over the loop variable, B023). It is now its own strict xfail, so fixing that bug turns it red and the marker is removed. - `style` is kept by the attrs default, so its case is gone. --- tests/test_xss.py | 120 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 100 insertions(+), 20 deletions(-) diff --git a/tests/test_xss.py b/tests/test_xss.py index 3c5c7f7..3ad3771 100644 --- a/tests/test_xss.py +++ b/tests/test_xss.py @@ -1,22 +1,25 @@ -"""Categorical XSS guard: render injection probes through each plugin's defaults. +"""XSS guard: render injection probes through plugins' default renderers (#143). 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). +the next unescaped interpolation, this drives each covered plugin end-to-end +with a probe and asserts the raw marker never reaches the output. -Two probe shapes cover the two sinks: +markdown-it is built with ``html=False``: the ``commonmark`` preset allows raw +HTML, which would pass the probes through core rules rather than a plugin. +Each case also states text the output must contain, which shows that the probe +reached the plugin's renderer (a payload the plugin does not parse would pass +the marker check without testing anything). -- ``">`` for HTML/attribute contexts: a plugin that escapes turns the ``<`` - into ``<`` so the ```` for element and attribute contexts: a plugin that escapes turns + the ``<`` into ``<``, so the ``)", - "{_TAG_ESCAPED}", ), Case( "dollarmath-label", lambda md: md.use(dollarmath_plugin), f"$$a=1$$ ({_TAG})\n", _TAG_MARKER, + f'
', + ), + Case( + "dollarmath-label-quote", + lambda md: md.use(dollarmath_plugin), + f"$$a=1$$ ({_QUOTE})\n", + _QUOTE_MARKER, + '
', ), Case( "myst-target", lambda md: md.use(myst_block_plugin), f"({_TAG})=\n", _TAG_MARKER, + f'', + ), + Case( + "myst-target-quote", + lambda md: md.use(myst_block_plugin), + f"({_QUOTE})=\n", + _QUOTE_MARKER, + '', + ), + Case( + "colon-fence-name", + lambda md: md.use(colon_fence_plugin), + f":::a{_TAG}\nx\n:::\n", + _TAG_MARKER, + f'', + ), + Case( + "colon-fence-name-quote", + # the block name ends at the first space, so the probe has none + lambda md: md.use(colon_fence_plugin), + ':::a"onxss="1\nx\n:::\n', + _QUOTE_MARKER, + '', + ), + Case( + "tasklists-label-after", + lambda md: md.use(tasklists_plugin, label=True, label_after=True), + f"- [ ] {_TAG}\n", + _TAG_MARKER, + f" {_TAG_ESCAPED}", ), Case( "attrs-span-event", lambda md: md.use(attrs_plugin, spans=True), '[click]{onxss="alert(1)"}\n', "onxss=", + "

click

", ), Case( "attrs-image-event", lambda md: md.use(attrs_plugin), '![a](img){onxss="alert(1)"}\n', "onxss=", + '

a

', ), Case( "attrs-block-event", lambda md: md.use(attrs_block_plugin), '{onxss="alert(1)"}\nparagraph\n', "onxss=", + "

paragraph

", + ), + Case( + "attrs-link-href-url", + lambda md: md.use(attrs_plugin), + '[a](https://x){href="javascript:alert(1)"}\n', + "javascript:", + '

a

', ), ] +def _md() -> MarkdownIt: + md = MarkdownIt("commonmark", {"html": False}) + md.options["xhtmlOut"] = False + return md + + @pytest.mark.parametrize("case", _CASES, ids=lambda case: case.id) def test_plugin_escapes_injection(case: Case) -> None: - md = MarkdownIt("commonmark") + md = _md() case.configure(md) out = md.render(case.payload) assert case.marker not in out, ( f"{case.id} emitted {case.marker!r} unescaped: {out!r}" ) + assert case.reached in out, f"{case.id} did not reach the renderer: {out!r}" + + +@pytest.mark.xfail( + strict=True, + reason="texmath never renders equation numbers: its render rules close over " + "the loop variable (# noqa: B023), so every block uses the last template", +) +def test_texmath_eqno_label_escaped() -> None: + """The texmath equation number is escaped when it is rendered. + + It is not rendered today, so this fails; when that bug is fixed it passes, + and ``strict`` turns the unexpected pass red so this marker is removed. + """ + md = _md().use(texmath_plugin, delimiters="gitlab") + src = f"```math x ``` ({_TAG})" + assert [t.type for t in md.parse(src)] == ["math_block_eqno"] + out = md.render(src) + assert _TAG_MARKER not in out + assert _TAG_ESCAPED in out From 03f3cfc441667f5f8a4caec413fe8dcdc166e3a5 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 9 Nov 2026 20:45:03 +0100 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=94=A7=20MAINTAIN:=20correct=20the?= =?UTF-8?q?=20attrs=20default-baseline=20test=20docstring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_attrs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_attrs.py b/tests/test_attrs.py index 8a0411c..c5d5bc4 100644 --- a/tests/test_attrs.py +++ b/tests/test_attrs.py @@ -101,7 +101,7 @@ def test_style_kept_by_default(src, token_type, expected): def test_benign_attrs_preserved_by_default(): - """The insecure-by-default baseline only strips on*, not benign attrs.""" + """The default baseline only strips on*, 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