fix(comark): denial-of-service hardening for the parser and stringifier - #375
Open
atinux wants to merge 9 commits into
Open
fix(comark): denial-of-service hardening for the parser and stringifier#375atinux wants to merge 9 commits into
atinux wants to merge 9 commits into
Conversation
Each non-void inline HTML opening tag recursed into the following tokens while looking for its matching close, so a line of 10k unclosed tags overflowed the call stack (RangeError) and aborted parsing. Thread a depth counter through processInlineToken and stop recursing at 100 levels (markdown-it's default maxNesting); deeper tags fall back to raw text, matching the unrecognized-tag path.
A fence info string like ```js {1-999999999} expanded every integer
in the range into an array — ~7s of CPU and ~2GB of heap from 20 bytes
of markdown, ending in RangeError. Bound both the width of a single
range and the total number of expanded lines (1000).
The parser JSON-decodes [...]/{...} attribute values, so
[x]{class=["a"]} stores class as an array and renderMarkdown crashed
with TypeError: value.split is not a function. Join arrays and fall
back to String() for other non-strings.
findParentListItem scanned all preceding tokens backwards for every inline token, so a document without any list (e.g. 80KB of plain paragraphs) cost O(n²) on the event loop — measured ~4s for ~78KB. Track open lists and list items with stacks during the single forward pass instead; the lookup becomes O(1) per inline token.
For every `<` in a text node, escapeInline re-tested /^[a-zA-Z!?/][^>]*>/ against the remainder of the node, scanning to the end when no `>` followed — a ~320KB text node of `<a` pairs blocked the event loop for ~44s in renderMarkdown. Precompute the next-`>` index once per node (one backwards pass) and check the tag shape in O(1) per occurrence.
…ding rule
The heading-with-attributes rule used .*?[ \t]* before the {…} block;
both fragments match spaces, so a heading followed by a long space run
and no attributes exploded combinatorially (~9s for 3000 spaces) when
the comark grammar tokenized md/markdown fences.
Make the title portion [^\n{]* — disjoint from ATTRS, so matching
stays linear per line.
A DAG-shaped json-render/yaml-render spec (each element listing the next one twice in children) was materialized recursively with no visited set, depth cap, or node budget: a ~1.4KB fence expanded to 2^24 AST nodes and exhausted the heap during parse. Cap expansion at 10k nodes and 100 levels; the existing try/catch leaves the fence as an inert code block when the budget trips.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
comark
@comark/angular
@comark/ansi
@comark/html
@comark/nuxt
@comark/react
@comark/svelte
@comark/vue
commit: |
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
Seven small, independent hardening fixes (one commit each): cap inline-HTML lookahead recursion, clamp codeblock highlight-range expansion, make the default task-list rule linear, make the
escapeInline<check linear, tolerate non-stringclassvalues incomarkAttributes, remove catastrophic backtracking in the rangi comark grammar, and bound json-render spec expansion.Why
A security scan found that small attacker-controlled markdown inputs could hang or crash the parsing/rendering process: 10k unclosed
<b>tags overflowed the call stack, a 20-byte{1-999999999}fence info allocated ~2GB, 78KB of plain paragraphs cost ~4s in the task-list rule, a 320KB run of<ablocked the event loop for ~44s inrenderMarkdown, a spaced-out heading line took ~9s in the rangi grammar, and a ~1.4KB json-render fence inflated to 2^24 AST nodes. Each fix now has a regression test built from the reported payload; no legitimate-document behavior changes (verified against the full SPEC suite).🤖 Prepared by an AI agent (OpenCode) from a security-audit findings list; commits are signed by the repository owner's key.