Skip to content

qmdb/sync/gaps: clamp checked_add to range.end instead of unwrap-panic#3706

Open
SAY-5 wants to merge 1 commit into
commonwarexyz:mainfrom
SAY-5:say5/gaps-checked-add-clamp-2068
Open

qmdb/sync/gaps: clamp checked_add to range.end instead of unwrap-panic#3706
SAY-5 wants to merge 1 commit into
commonwarexyz:mainfrom
SAY-5:say5/gaps-checked-add-clamp-2068

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 2, 2026

Fixes #2068.

Location::checked_add returns None on overflow OR when the result exceeds Family::MAX_LEAVES. The two callsites in gaps::find_next unconditionally .unwrap()'d the result, so fuzzer inputs with start_loc near the family ceiling panicked the sync engine.

Clamp to range.end on None so the merge loop terminates on the next iteration as expected, instead of crashing.

Comment thread storage/src/qmdb/sync/gaps.rs Outdated
Comment on lines +40 to +46
// `Location::checked_add` returns `None` on overflow *or* when the result
// exceeds `Family::MAX_LEAVES`. The previous unconditional `.unwrap()`
// panicked on fuzzer-generated inputs that placed `start_loc` near the
// ceiling. Clamp such ranges to `range.end` instead — semantically the
// batch is already at the boundary, so the merge logic terminates the
// loop on the next iteration as expected (#2068).
let range_end = range.end;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is cleaner with just the .unwrap_or changes.

Suggested change
// `Location::checked_add` returns `None` on overflow *or* when the result
// exceeds `Family::MAX_LEAVES`. The previous unconditional `.unwrap()`
// panicked on fuzzer-generated inputs that placed `start_loc` near the
// ceiling. Clamp such ranges to `range.end` instead — semantically the
// batch is already at the boundary, so the merge logic terminates the
// loop on the next iteration as expected (#2068).
let range_end = range.end;

@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 11, 2026

Agreed, I'll trim the diff to just the unwrap_or adjustments and push.

Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
@SAY-5 SAY-5 force-pushed the say5/gaps-checked-add-clamp-2068 branch from a1878a5 to 9db797e Compare May 11, 2026 17:47
@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 11, 2026

Pushed 9db797e, trimmed the diff to just the two .unwrap_or(range.end) replacements.

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.

panicking at storage/src/adb/sync/gaps.rs:51

2 participants