Repository navigation
verifier: propagate max_depth and max_tables budgets across VerifyNestedFlatBuffer - #9418
Open
professor-moody wants to merge 2 commits into
Open
professor-moody wants to merge 2 commits into
professor-moody wants to merge 2 commits into
Conversation
…tedFlatBuffer ## Summary This PR propagates the active `max_depth` and `max_tables` verifier budgets across `VerifierTemplate::VerifyNestedFlatBuffer` (`include/flatbuffers/verifier.h`) and adds regression coverage in `tests/monster_test.cpp` (`Tests/NestedVerifierBudgetExhausted`): 1. **Propagate `max_depth` and `max_tables` across `VerifyNestedFlatBuffer` (`include/flatbuffers/verifier.h`)**: - Previously, `VerifyNestedFlatBuffer<T>` constructed a fresh `VerifierTemplate<TrackVerifierBufferSize>` with the caller's unmodified `opts_` (`max_depth = 64`, `max_tables = 1000000`) and did not fold `nested_verifier.num_tables_` back into the outer verifier. For schemas with self-referential `nested_flatbuffer` fields (such as `MyGame::Example::Monster::testnestedflatbuffer`), a chain of nested FlatBuffers reset `depth_ = 0` and `num_tables_ = 0` at every hop, bypassing `opts_.max_depth` and `opts_.max_tables` and allowing unbounded recursion (`depth >= 52,000` in ASan, `100,000` in Release `-O3`) that terminates in stack exhaustion (`SIGSEGV`) inside `VerifyMonsterBuffer()`. - `VerifyNestedFlatBuffer` now checks `depth_ < opts_.max_depth` and `num_tables_ <= opts_.max_tables`, initializes `nested_opts.max_depth = opts_.max_depth - depth_` and `nested_opts.max_tables = opts_.max_tables - num_tables_`, and accumulates `num_tables_ += nested_verifier.num_tables_` on return. 2. **Regression Test (`tests/monster_test.cpp`)**: - Adds `Tests/NestedVerifierBudgetExhausted` (`NestedVerifierBudgetExhausted()`), verifying that a 2-level nested `Monster` buffer passes verification while nested chains exceeding `opts.max_depth` or cumulative `opts.max_tables` are cleanly rejected. ## Verification - Built and tested against `master` (`b8431fbcd7a5c71817f314e18b332c0648554efa`): - `ctest` (`flattests`) `1/1` passed in both Release (`-O3 -DNDEBUG`) and ASan+UBSan (`-O1 -g -fsanitize=address,undefined`). - Confirmed that deeply nested `testnestedflatbuffer` chains (52,000 levels under ASan; 100,000 levels in Release `-O3`) that previously caused stack overflow (`SIGSEGV`) in `VerifyMonsterBuffer()` are now cleanly rejected (`VerifyMonsterBuffer() == false`) once the cumulative depth or table budget is reached.
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
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
This PR propagates the active
max_depthandmax_tablesverifier budgets acrossVerifierTemplate::VerifyNestedFlatBuffer(include/flatbuffers/verifier.h) and wires regression coverage into the main test suite (tests/monster_test.h,tests/monster_test.cpp,tests/test.cpp):Propagate
max_depthandmax_tablesacrossVerifyNestedFlatBuffer(include/flatbuffers/verifier.h):VerifyNestedFlatBuffer<T>constructed a freshVerifierTemplate<TrackVerifierBufferSize>with the caller's unmodifiedopts_(max_depth = 64,max_tables = 1000000) and did not foldnested_verifier.num_tables_back into the outer verifier. For schemas withnested_flatbufferfields (such asMyGame::Example::Monster::testnestedflatbuffer), nested FlatBuffers resetdepth_ = 0andnum_tables_ = 0at every hop, bypassingopts_.max_depthon deep chains andopts_.max_tablesacross both chained and sibling nested buffers.VerifyNestedFlatBuffernow checksdepth_ < opts_.max_depthandnum_tables_ <= opts_.max_tables, initializesnested_opts.max_depth = opts_.max_depth - depth_andnested_opts.max_tables = opts_.max_tables - num_tables_, and after successful nested verification accumulatesnum_tables_ += nested_verifier.num_tables_and checksnum_tables_ <= opts_.max_tables.Regression Test (
tests/monster_test.h,tests/monster_test.cpp,tests/test.cpp):flatbuffers::tests::NestedVerifierBudgetExhausted()intests/monster_test.h, implements it intests/monster_test.cpp, and invokes it fromFlatBufferTestsintests/test.cpp.Monstercontrol passes, a 32-level chain is rejected whenopts.max_depth = 16(while passing under defaultmax_depth = 64), a 12-level chain is rejected whenopts.max_tables = 8, and 3 sibling nestedMonsterbuffers undertestarrayoftables(13 cumulative tables, max single-branch table count 7) are rejected whenopts.max_tables = 8.Verification
flattestsagainstmaster(b8431fbcd7a5c71817f314e18b332c0648554efa, cleanly mergeable withf58d5768f1fd0df9f71aa6ae5f63645e2637c31f):include/flatbuffers/verifier.hfix,flattestsfails all three budget assertions (depth_limited,table_limited, andsibling_limited) inNestedVerifierBudgetExhausted().flattestspasses (ALL TESTS PASSED) in both Release and ASan+UBSan builds.