Fix crack_row_jet4 overflow panic when var-offset table wraps#8
Open
Bradyok wants to merge 1 commit intodominion525:mainfrom
Open
Fix crack_row_jet4 overflow panic when var-offset table wraps#8Bradyok wants to merge 1 commit intodominion525:mainfrom
Bradyok wants to merge 1 commit intodominion525:mainfrom
Conversation
When a row's declared var_col_count claims more variable columns than physically fit before vcc_pos, the backward-walking offset table loop computes `pos = vcc_pos.wrapping_sub(2 + i * 2)` which wraps to a near-`usize::MAX` value. The bounds check that follows was written as `if pos + 2 > len` — the plain `pos + 2` overflow- panics in debug before the `> len` comparison can reject the wrapped value. Swap the unchecked add for `checked_add(2)` so the bounds check itself is panic-safe; a wrapped `pos` now breaks cleanly out of the loop. Triggered in the wild by Hy-Tek Meet Manager 8 .mdb files — specifically the `ScoringImprovement` and `ScoreLanes` tables. Adds a targeted regression test against a synthetic row that declares `var_col_count = 100` in a 7-byte buffer; the old code panicked with `attempt to add with overflow`, the new code returns `Ok` with as many offsets as actually fit.
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
crack_row_jet4(crates/jetdb/src/data.rs:243) panics in debug builds withattempt to add with overflowon rows where the declaredvar_col_countclaims more variable columns than physically fit beforevcc_pos.posis already computed withwrapping_sub(correct), but the very next line —if pos + 2 > len— does a plainusize + 2, and whenposhas wrapped to nearusize::MAXthat add itself panics before the length check can reject it.Reproducer
Any
.mdbwhose variable-length offset table would extend past the start of the row. I hit it in the wild on Hy-Tek Meet Manager 8.mdbexports — specifically theScoringImprovementandScoreLanestables of a 2025 meet file.The included regression test (
crack_row_jet4_bogus_var_col_count_does_not_overflow) builds a 7-byte synthetic row withvar_col_count = 100which forces the loop well pastvcc_pos. Without the fix it panics on the third iteration; with the fix it cleanly breaks out of the loop and returnsOkwith the two valid offsets that actually fit.Fix
Swap
pos + 2 > lenforpos.checked_add(2)so the bounds check itself is panic-safe:Semantically equivalent for every non-wrapped
pos; the only difference is that a wrappedposnow breaks out of the loop instead of overflow-panicking in debug.Verification
cargo test -p jetdb --lib— 620 tests pass (was 619, +1 for the new regression test)cargo test -p jetdb --lib data::tests::crack_row_jet4— 6/6 passHappy to adjust the test fixture, split the test into its own
#[test], or add a fuzz-target-style input if you'd prefer.