Skip to content

Validate serialized scanner state before restoring it - #5

Open
tarebyte wants to merge 2 commits into
masterfrom
fix/scanner-deserialize-validation
Open

Validate serialized scanner state before restoring it#5
tarebyte wants to merge 2 commits into
masterfrom
fix/scanner-deserialize-validation

Conversation

@tarebyte

@tarebyte tarebyte commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Problem

deserialize() in src/scanner.c read the serialized scanner state without bounds checking any field. Tree-sitter hands back whatever buffer it was given, so those bytes are untrusted input.

The worst case is the heredoc record, which carries a 32-bit end-word length that went straight to array_reserve and memcpy with no comparison against the remaining buffer:

uint32_t word_length;
memcpy(&word_length, &buffer[size], sizeof(uint32_t));
size += sizeof(uint32_t);
array_reserve(&heredoc.word, word_length);
memcpy(heredoc.word.contents, &buffer[size], word_length);

The only size check was assert(size == length) at the very end — after every read had already happened.

Change

Bounds-checks every field read: the leading byte, each 5-byte literal record, the heredoc fixed fields, the length-prefixed end word, and the trailing bytes. Any inconsistency calls reset() and returns, so the scanner starts from an empty state rather than restoring garbage.

Literal records are also checked against the token types that can actually reach the literal stack. Those are the delimited-literal opening tokens, which form one contiguous run in TokenType; rejecting only out-of-enum values would still admit records describing states the scanner cannot produce, such as a literal whose type is LINE_BREAK or NONE.

Scope of the validation

This rejects buffers that are structurally impossible: wrong length, unconsumed trailing bytes, an end word longer than the buffer, or a literal type that cannot occur. It does not verify the remaining field values — delimiters, nesting depth, and the boolean flags are accepted as-is, so a hand-crafted buffer of the right shape can still restore a nonsensical-but-harmless state. The goal here is memory safety, not full state validation.

Validation

  • test_external_scanner_rejects_malformed_state drives 20,000 pseudo-random buffers through deserialize and re-serializes each one. Against master it fails, in both debug and release, with Assertion failed: (size == length) ... signal: 6, SIGABRT. It passes on this branch.

    Note that the assertion is what surfaces the corruption here — cargo does not define NDEBUG, so it is present in release builds too. A consumer that compiles the scanner with NDEBUG loses it and gets the out-of-bounds read silently, which is the case this PR is really about.

  • Building master with -fsanitize=address,undefined and running the same deterministic 20,000-buffer sequence reports ERROR: AddressSanitizer: stack-buffer-overflow ... in deserialize. The identical run on this branch is clean.

    The read length depends on the bytes that land in the word_length field and varies per seed. Most stop at the first byte past the buffer; the largest observed across the seeds tried was a 2,985,668,234-byte read, which is simply the old code trusting the full uint32_t.

  • test_external_scanner_round_trips_well_formed_state and test_external_scanner_rejects_impossible_literal_type call the scanner's serialize/deserialize entry points directly. They confirm that a valid string-literal state and a valid started-heredoc state come back byte-identical, that every acceptable literal type round-trips, and that LINE_BREAK, COMMENT, UNINTERPRETED and NONE reset instead. These are the tests that show validation does not discard real state.

  • 332 corpus parses and 5 highlight files, zero failures. All 4,437 examples/ruby_spec files produce byte-identical trees to master. Both are regression checks: a from-scratch parse calls deserialize with a zero length, so neither exercises the changed code.

  • CI already runs the Rust tests (test-rust: true in .github/workflows/ci.yml).

References

Hardens the code path in tree-sitter#269, where the same assertion fires while parsing a heredoc, and covers the malformed-state handling proposed in tree-sitter#297.

Merge note

This and #12 both add tests to the same module in bindings/rust/lib.rs and will conflict; whichever lands second needs the other's test block re-applied.

`deserialize` read the serialized scanner state without any bounds
checking and only sanity-checked the result with an `assert` that is
compiled out of release builds. Any buffer that did not exactly match
the layout `serialize` produces was restored anyway, and the 32-bit
heredoc word length was passed straight to `memcpy`, so a malformed or
truncated buffer could read gigabytes past the end of the caller's
buffer.

Bounds-check every field against the remaining input, reject
out-of-range literal token types, and reset the scanner to its empty
state whenever the buffer does not describe a complete, exactly-consumed
state. A stale or corrupt buffer now degrades to a fresh scan instead of
corrupting memory.

Add a Rust test that feeds pseudo-random buffers through
deserialize/serialize; it crashes with SIGBUS against the previous
implementation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant