Repository navigation
Conversation
A table with a union field whose type is set but whose value is absent passed VerifyBuffer. The generated union verify case called verifier.VerifyTable(ptr), which accepts a null pointer, and the object API then dereferenced that null pointer when the buffer was packed again. Check the table pointer for table union members, regenerate the affected checked-in headers, and add a regression test. Fixes google#9416
Author
|
Issue #9416 is also addressed by #9417, which adds a null guard at the start of the generated Pack functions (if (!value) { return 0; }). This PR changes the generated Verify functions so that a union whose type is set but whose value is missing is rejected during verification (return ptr && verifier.VerifyTable(ptr);). The two changes do not conflict in intent: the verifier change rejects the malformed buffer, and the Pack guard protects callers of Pack(). We are happy to rebase onto whichever approach the maintainers prefer, or to combine them. Please let us know which you would like to keep. |
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.
Fixes #9416.
Problem
A table with a union field whose type is set but whose value is absent passes
VerifyBuffer. The generated union verify case callsverifier.VerifyTable(ptr), andVerifyTableaccepts a null pointer. The generated object API then dereferences that null pointer when the buffer is unpacked and packed again.Reproduced locally on the unpatched tree with clang on Linux:
VerifyMonsterBufferreturns true, and the object-API round trip ends in a SIGSEGV inMonster::Packwith_o == nullptr. An AddressSanitizer/UBSan build reports the same null member access at the same generated line.Change
src/idl_gen_cpp.cpp: for union members that are tables, the generator now emitsreturn ptr && verifier.VerifyTable(ptr);.flatcusingscripts/generate_code.py. Every changed line is the union verify line; no other line changed.tests/union_underlying_type_test_generated.hhas the same one-line change applied by hand. I could not identify the command that generates this file (it is not inscripts/generate_code.py), so please check it against the generator if you have that command.tests/test.cpp: newUnionTypeWithoutValueRejectedTest. It checks that a union type with no value is rejected, and that a union with a value, and a union of type NONE, still verify.Verification
flattestssuite passes (build exit 0, run exit 0).test.cpp: the new test fails attest.cpp(the check that the union-without-value buffer is rejected).Scope and limits
include/flatbuffers/verifier.h;VerifyTablestill accepts null for optional fields.