Repository navigation
🔒 FIX: escape colon_fence class and tasklists label_after content - #165
Merged
Merged
Conversation
The default renderer interpolated the block name into `class="block-..."` unescaped, after unescaping entities in the info string, so `:::a"onmouseover="alert(1)` (or its entity-encoded form) injected attributes and markup even with `html=False` (#143).
With `label=True, label_after=True` the label was built from the item's source text and emitted as an `html_inline` token, so `- [ ] <img src=x onerror=alert(1)>` rendered a raw `<img>` even with `html=False` (#143). The label text is now HTML-escaped.
A colon_fence block name containing '&' now emits '&' (same attribute value), and a tasklists label_after source entity is shown as written.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #165 +/- ##
==========================================
+ Coverage 92.80% 93.30% +0.49%
==========================================
Files 31 40 +9
Lines 1835 2285 +450
==========================================
+ Hits 1703 2132 +429
- Misses 132 153 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chrisjsewell
added a commit
that referenced
this pull request
Nov 9, 2026
- 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 `"><xss>` 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Two more HTML-injection points in the #143 family, both reachable with
html=False:class="block-..."unescaped,after entities in the info string had been unescaped.
:::a"onmouseover="alert(1)renderedclass="block-a"onmouseover="alert(1)",and
:::a"><xss>rendered a raw<xss>element.The name is now passed through
escapeHtml.label=True, label_after=True: the label is built from the item's source textand emitted as an
html_inlinetoken, so- [ ] <img src=x onerror=alert(1)>rendered a raw<img>.The label text is now passed through
escapeHtml.How it was tested
html=Falseand exact expected output:tests/test_colon_fence.py::test_block_name_is_escaped(raw-quote and entity-encoded payloads, plus a benign&)and
tests/test_tasklists.py::test_label_after_is_escaped(raw HTML,&, plain text),with
test_label_after_entity_is_shown_literallypinning how a source entity appears in the label(it asserts the label only, as earlier inline children of a
label_afteritem are also rendered outside it,an existing quirk not changed here).
reverting either source file at HEAD makes its tests fail again.
pytest -q: 518 passed (511 on master plus the 7 new cases), markdown-it-py 4.2.0, Python 3.12.pre-commit run --all-files(ruff, ruff-format, mypy): clean.Part of #143
This does not close #143. What remains of it is in other open PRs:
the texmath, dollarmath and myst_block escaping and the
attrsdefault (#150),the ReDoS fixes (#148),
and a test that guards every plugin renderer against XSS (#151).
Notes for the reviewer
a colon_fence block name containing
&now emits&(class="block-a&b"instead ofclass="block-a&b"),which is the same attribute value;
and in tasklists
label_aftermode, label text containing&,<,>or"is now escaped (
a & bbecomesa & b, which is correct HTML).The label is the item's raw source text (it already showed
*emphasis*literally),so a source entity such as
&now displays as the literal text&.This applies with
html=Trueas well; on master, inline HTML in alabel_afteritem already produced broken markup.No existing fixture uses
label_after.tests/test_xss.py(which arrives with ✅ TEST(#143): add categorical XSS guard across plugin renderers #151) is left for after those PRs land.