Bound FlexBuffers verifier work for shared references - #9197
Open
carrerasdarren-cell wants to merge 1 commit into
Open
Bound FlexBuffers verifier work for shared references#9197carrerasdarren-cell wants to merge 1 commit into
carrerasdarren-cell wants to merge 1 commit into
Conversation
Cap aggregate vector traversal and key scanning relative to buffer size and maximum depth so shared references cannot amplify default verification into an excessive CPU cost. Keep heavily shared DAGs supported through the existing reuse tracker and add regression coverage for both paths.
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.
Summary
Fixes #9127.
Problem
The public OSS-Fuzz testcase for issue 522117389 contains a 131,069-byte
FlexBuffer nested in a FlatBuffer. Repeated references cause the default
verifier to revisit the same 1,028-element key vector and rescan long key
suffixes many times.
On current
master, a release build took 8.72 seconds inVerifierand another8.72 seconds in
SizeVerifieron the testcase. The ASan/UBSan reproducer wasstill running after 60 seconds. Sampling placed essentially all CPU time in
Verifier::VerifyKeythrough the repeated shared vector.The existing reuse tracker avoids this amplification, but it is optional and
the default verifier is commonly invoked on untrusted buffers without one.
Fix
Account for each traversed vector element and scanned key byte. The default
budget is proportional to
buffer size * max depth, with saturation onoverflow. This bounds default verification while leaving room for ordinary
sharing. Callers that expect heavily shared DAGs can continue to supply the
existing reuse tracker; repeated values then short-circuit before consuming
additional work.
Key termination checks use
memchrbounded by the remaining work budget, so asingle long key cannot overrun the limit before it is checked.
Validation
flattests:ALL TESTS PASSEDverifier_fuzzerreproducer: clean exit in 0.26 secondsdefault and accepted when the reuse tracker is supplied
git clang-format --diff: cleangit diff --check: clean