Skip to content

fix: guard empty ObjectScript scanner state - #2800

Merged
DeusData merged 2 commits into
DeusData:mainfrom
WarGloom:fix/objectscript-empty-deserialize
Oct 27, 2026
Merged

DeusData merged 2 commits into
DeusData:mainfrom
WarGloom:fix/objectscript-empty-deserialize

Conversation

@WarGloom

@WarGloom WarGloom commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #2799. Guard zero-length state copies in the vendored ObjectScript routine and UDL external-scanner deserializers. Tree-sitter calls these functions with a NULL buffer and zero length when there is no external scanner state; the unconditional memcpy triggers UBSan even though it copies no bytes. Nonempty copies are unchanged.

Add a regression case for each scanner to the existing grammar regression suite: accept (NULL, 0), preserve fresh state, and round-trip a nonempty serialized state. Document both local patches for re-vendoring and refresh the three affected vendored-file checksums (two scanners plus manifest).

Overlap: #2368 already includes equivalent guards as part of a larger grammar re-vendor for Python bodies. This PR isolates the sanitizer correction and regression coverage so it can be reviewed and merged independently; it does not claim a separate discovery or replace that grammar update.

Validation

  • Reproduced on main 6efe33b7: each actual upstream scanner compiled with -fsanitize=undefined -fno-sanitize-recover=all exits 1 on deserialize(payload, NULL, 0) with null pointer passed as argument 2 (routine line 118, UDL line 122).
  • Both guarded scanners pass that call and a nonempty state round-trip with the same UBSan settings (exit 0).
  • Passed scripts/lint.sh --ci with Cppcheck 2.20.0, scripts/security-vendored.sh, scripts/security-audit.sh, test-source clang-format, and git diff --check. Passed ASAN_OPTIONS=detect_leaks=0:detect_stack_use_after_return=0 scripts/test.sh --suites grammar_regression,grammar_labels,grammar_imports CC=clang CXX=clang++: 6 passed, exit 0. This includes both new scanner tests and the existing per-language extraction, label, and import checks. The full unit suite was not run locally; CI remains pending.

AI assistance: OpenAI Codex assisted with investigation, implementation, tests, documentation, and validation on behalf of WarGloom. The submitting contributor is responsible for this change.

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally (make -f Makefile.cbm test; focused validation specified above)
  • Lint passes (scripts/lint.sh --ci)
  • New behavior is covered by regression tests; original scanner failure reproduced before validation

Signed-off-by: wargloom <wargloom@gmail.com>
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@WarGloom
WarGloom marked this pull request as ready for review October 25, 2026 18:46
@WarGloom
WarGloom requested a review from DeusData as a code owner October 25, 2026 18:46
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you, @WarGloom, and thanks again for #2737 earlier today! This one reviewed cleanly: the guard has exactly the shape we already carry for the crystal, rescript and purescript scanners, both local patches are recorded in MANIFEST.md, the three checksum pins match the new bytes exactly, and the two regression tests cover both halves of the contract. Another grammar regression test landed in the same test file today, so we merged current main into your branch (one signed-off merge commit, eaed3cd; your commit is unchanged). Please pull before pushing to it again. We will merge once the fresh checks are green.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges labels Oct 25, 2026
@DeusData
DeusData merged commit 5948316 into DeusData:main Oct 27, 2026
41 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged as 5948316 - thank you, @WarGloom! Guarding the empty ObjectScript scanner state removes the crash path from #2799, and splitting it out of the larger re-vendor made it easy to land on its own. Thank you for the careful, focused change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ObjectScript scanners pass a null empty-state buffer to memcpy

2 participants