Conversation
1. In flexbuffers.h, Verifier::VerifyBuffer() performed pointer arithmetic (buf_ + size_) and immediately dereferenced *--end without validating that buf_ is non-null. When a null buffer is passed with size_ >= 3, this leads to a segmentation fault rather than returning false. Additionally, flexbuffers::VerifyBuffer() instantiated the verifier and attempted to clear and resize reuse_tracker_ even when buf is NULL. Add null checks in Verifier::VerifyBuffer(), flexbuffers::VerifyBuffer(), Verifier::VerifyFrom, Verifier::VerifyBefore, and guard reuse_tracker_ initialization. 2. In verifier.h, VerifierTemplate::Verify() and VerifyBufferFromStart() checked buffer bounds against size_ but did not verify that buf_ is non-null. When a Verifier is initialized with a null buffer pointer and VerifyBuffer<T>() is invoked, Verify() returned true for initial offsets, causing ReadScalar<OffsetT>() to dereference a null pointer. Add buf_ != nullptr checks to VerifierTemplate::Verify() and VerifyBufferFromStart(). 3. In tests/flexbuffers_test.cpp and tests/test.cpp, add regression tests verifying that passing null buffer pointers to flexbuffers::VerifyBuffer() and flatbuffers::Verifier gracefully returns false without crashing. Signed-off-by: Filippo Tedeschi <filippotedeschi98@gmail.com>
This branch has not been deployed
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
1 participant
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.
Description
This PR strengthens NULL pointer safety and defensive checks across
flatbuffers::Verifierandflexbuffers::VerifyBuffer:flexbuffers.h:Verifier::VerifyBuffer(), pointer arithmeticauto end = buf_ + size_;followed byauto byte_width = *--end;dereferenced unmapped memory whenbuf_ == nullptrandsize_ >= 3, leading to a hardware segmentation fault instead of gracefully returningfalse. AddedCheck(buf_ != nullptr && size_ >= 3).flexbuffers::VerifyBuffer(), addedif (!buf) return false;to fail immediately and avoid unneeded allocations inreuse_tracker_.Verifier::VerifyFromandVerifier::VerifyBefore, addedbuf_ != nullptrchecks to ensure range validation never succeeds on a null buffer pointer.reuse_tracker_initialization inVerifier::Verifier()withif (buf_ && reuse_tracker_).verifier.h:VerifierTemplate::Verify(), addedbuf_ != nullptrto range verification. Previously,Verify()evaluated only bounds againstsize_, causingVerifyOffset()to succeed on offset 0 for a null buffer and subsequently dereferencenullptrinReadScalar<OffsetT>(buf_ + start).VerifyBufferFromStart(), addedbuf_ != nullptrcheck in the minimum buffer size guard:Check(buf_ != nullptr && size_ >= FLATBUFFERS_MIN_BUFFER_SIZE).Regression Tests:
tests/flexbuffers_test.cppandtests/flexbuffers_test.h, addedFlexBuffersVerifyNullBufferTest()validating that passing null buffers (both with and withoutreuse_tracker) returnsfalsewithout crashing.tests/test.cpp, addedVerifierNullBufferTest()verifying that passing a null buffer toflatbuffers::VerifierreturnsfalseinVerifyBuffer<Monster>()andVerify().Signed-off-by: Filippo Tedeschi filippotedeschi98@gmail.com