Fix capacity-overflow panic on untrusted counts in helpers::count - #369
Open
gaoflow wants to merge 1 commit into
Open
Fix capacity-overflow panic on untrusted counts in helpers::count#369gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
The Vec<u8> fast path reserved the input-driven count before reading any byte, so a count of u64::MAX (8 bytes of 0xFF) panicked with "capacity overflow" in Vec::reserve_exact. Drop the reserve and let read_to_end grow the Vec as bytes are read, matching the chunked pattern used by read_vec_fast_int and the hardening applied to punctuated in jam1garner#365.
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.
helpers::countreservesVec<u8>capacity from the input-driven count, so acount of
u64::MAX(8 bytes of0xFF) panics with "capacity overflow" beforeany byte is read. Every
#[br(count = N)] Vec<u8>field routes through thispath.
#365 hardened
punctuatedagainst this class ("Let the Vec grow as elementsare successfully read, like
helpers::count"), buthelpers::count's ownVec<u8>fast path is the un-hardened sibling. The reserve is dropped so theVec grows as bytes are read, the same std exponential strategy
read_vec_fast_intalready uses for integer vectors.
panics before this change (debug and release); after it,
Err(not enough bytes).Regression test covers the huge-count case, short input, count=0, and a valid
1 MiB read.