Skip to content

Fix tree-sitter-markdown scanner serialize buffer overflow#60312

Merged
Veykril merged 4 commits into
mainfrom
richard/fr-115-tree-sitter-markdown-scanner-serialize-buffer-overflow
Jul 7, 2026
Merged

Fix tree-sitter-markdown scanner serialize buffer overflow#60312
Veykril merged 4 commits into
mainfrom
richard/fr-115-tree-sitter-markdown-scanner-serialize-buffer-overflow

Conversation

@rtfeldman

@rtfeldman rtfeldman commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Zed bundles the markdown grammar's block scanner natively, and its serialize() memcpys the open-block stack into tree-sitter's fixed 1024-byte serialization buffer with no bounds check. Markdown with roughly 255+ nested blocks overflows that buffer, and because it sits at the front of struct TSParser, the overflow clobbers the adjacent parse-stack pointer and heap. Debug builds of the tree-sitter runtime catch this with an assertion, but release builds like Zed's have no check and silently corrupt parser memory — which is why this surfaced as wild crashes deep in tree-sitter's parse stack rather than clean failures. Still open upstream as tree-sitter-grammars/tree-sitter-markdown#243.

This PR points tree-sitter-md at a zed-industries fork whose serialize() refuses to write state that doesn't fit (bounded by the same running counter the header writes advance, so the check can't drift). The scanner then deserializes to a fresh state, and the pathologically nested region surfaces as ordinary tree-sitter ERROR nodes — visible, safe degradation for an adversarial input class, deliberately chosen over the two alternatives: truncating the block stack would deserialize into a plausible-but-wrong state and produce silently incorrect trees, and the scanner ABI offers no error channel at all (serialize() returns a length into a fixed buffer; there is no way to fail a parse). A nesting-depth cap at block-open time would give fully deterministic semantics, but that's a behavior change across 13 scanner call sites that belongs upstream, not in a hotfix fork.

The pinned branch is upstream's 9a23c1a9 (the revision Zed already pinned) plus exactly two commits, for easy review: the guard (zed-industries/tree-sitter-markdown@179422e) and regression tests (zed-industries/tree-sitter-markdown@b596e73). The deep-nesting test aborts on the unguarded scanner and passes with the guard; a moderate-nesting test pins that inputs fitting the buffer still parse cleanly. The same change is also up as zed-industries/tree-sitter-markdown#1 into the fork's default branch (split_parser), so future pin bumps don't lose it; if upstream fixes #243, we can drop the fork entirely on the next bump.

Closes FR-115

Release Notes:

  • Fixed a potential crash when editing Markdown with deeply nested blocks

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Jul 2, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Jul 2, 2026
@rtfeldman rtfeldman force-pushed the richard/fr-115-tree-sitter-markdown-scanner-serialize-buffer-overflow branch 2 times, most recently from 40f4165 to 8ce79f9 Compare July 6, 2026 14:16
rtfeldman and others added 4 commits July 6, 2026 21:40
Point tree-sitter-md at a zed-industries fork of the pinned revision that
guards serialize() against overflowing tree-sitter's fixed serialization
buffer when many markdown blocks are open (deep nesting). The overflow
previously clobbered the parser's adjacent Stack pointer and heap.

Upstream report: tree-sitter-grammars/tree-sitter-markdown#243
…teral

Updates the fork pin: the overflow check now derives its header size from
the same size counter the header writes advance, instead of a hardcoded 5
that could silently drift if header fields are added.
@rtfeldman rtfeldman force-pushed the richard/fr-115-tree-sitter-markdown-scanner-serialize-buffer-overflow branch from 8ce79f9 to 06c1e18 Compare July 7, 2026 01:41
@Veykril Veykril added this pull request to the merge queue Jul 7, 2026
@Veykril

Veykril commented Jul 7, 2026

Copy link
Copy Markdown
Member

/cherry-pick preview

Merged via the queue into main with commit 28c2e7d Jul 7, 2026
34 checks passed
@Veykril Veykril deleted the richard/fr-115-tree-sitter-markdown-scanner-serialize-buffer-overflow branch July 7, 2026 07:31
@zed-zippy

zed-zippy Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/28849425897

TakuroBreath pushed a commit to TakuroBreath/zed that referenced this pull request Jul 7, 2026
…tries#60312)

Zed bundles the markdown grammar's block scanner natively, and its
`serialize()` `memcpy`s the open-block stack into tree-sitter's fixed
1024-byte serialization buffer with no bounds check. Markdown with
roughly 255+ nested blocks overflows that buffer, and because it sits at
the front of `struct TSParser`, the overflow clobbers the adjacent
parse-stack pointer and heap. Debug builds of the tree-sitter runtime
catch this with an assertion, but release builds like Zed's have no
check and silently corrupt parser memory — which is why this surfaced as
wild crashes deep in tree-sitter's parse stack rather than clean
failures. Still open upstream as
tree-sitter-grammars/tree-sitter-markdown#243.

This PR points `tree-sitter-md` at a `zed-industries` fork whose
`serialize()` refuses to write state that doesn't fit (bounded by the
same running counter the header writes advance, so the check can't
drift). The scanner then deserializes to a fresh state, and the
pathologically nested region surfaces as ordinary tree-sitter `ERROR`
nodes — visible, safe degradation for an adversarial input class,
deliberately chosen over the two alternatives: truncating the block
stack would deserialize into a plausible-but-wrong state and produce
silently incorrect trees, and the scanner ABI offers no error channel at
all (`serialize()` returns a length into a fixed buffer; there is no way
to fail a parse). A nesting-depth cap at block-open time would give
fully deterministic semantics, but that's a behavior change across 13
scanner call sites that belongs upstream, not in a hotfix fork.

The pinned branch is upstream's `9a23c1a9` (the revision Zed already
pinned) plus exactly two commits, for easy review: the guard
(zed-industries/tree-sitter-markdown@179422e)
and regression tests
(zed-industries/tree-sitter-markdown@b596e73).
The deep-nesting test aborts on the unguarded scanner and passes with
the guard; a moderate-nesting test pins that inputs fitting the buffer
still parse cleanly. The same change is also up as
zed-industries/tree-sitter-markdown#1 into the fork's default branch
(`split_parser`), so future pin bumps don't lose it; if upstream fixes
zed-industries#243, we can drop the fork entirely on the next bump.

Closes FR-115

Release Notes:

- Fixed a potential crash when editing Markdown with deeply nested
blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants