fix: add GenTextWithVerify overload to prevent OOB reads in text gen#9156
Open
samrigby64 wants to merge 1 commit into
Open
fix: add GenTextWithVerify overload to prevent OOB reads in text gen#9156samrigby64 wants to merge 1 commit into
samrigby64 wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
GenText / GenerateText traverse FlatBuffer offsets and union-type-vector pointers without re-verifying them against the buffer boundaries: * PrintVector (idl_gen_text.cpp ~166): calls vec.size() on a Vector<T>* cast from an unverified field offset. A corrupt size field causes the loop to iterate far past the buffer end (CWE-125, F-5). * PrintOffset BASE_TYPE_UNION vector path (~190): follows prev_val + ReadScalar<uoffset_t>(prev_val) without bounds-checking the resulting type_vec pointer, which can point outside the buffer on malformed input (CWE-125, F-6). Changes: - Add GenTextWithVerify(parser, buf, buf_size, text) to idl.h and idl_gen_text.cpp. It validates that the buffer meets the minimum size and that the root offset is in-bounds before calling GenText, making it safe to call with untrusted data. - Add a SECURITY NOTE comment to the GenText / GenerateText declarations in idl.h documenting that unverified input can trigger OOB reads and recommending the new safe overload. Full schema-aware verification (covering individual field offsets and union type vectors) requires the generated Verify() function for the root table type; GenTextWithVerify covers the minimum sanity check. Reported by: Sam Rigby (samrigby432@outlook.com)
a882134 to
2c9b59c
Compare
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.
Problem
GenText/GenerateTexttraverse FlatBuffer offsets and union-type-vector pointers without re-verifying them against the buffer boundaries:PrintVector(idl_gen_text.cpp~line 160) callsvec.size()on aVector<T>*cast from an unverified field offset. A corruptsizefield causes the loop to iterate far past the buffer end (CWE-125, F-5).PrintOffset/ BASE_TYPE_UNION vector path (idl_gen_text.cpp~line 190) followsprev_val + ReadScalar<uint8_t>(prev_val)without bounds-checking the resulting type-vec pointer, which can point outside the buffer on malformed input (CWE-125, F-6).Changes
GenTextWithVerify(parser, buf, size, text)toidl.handidl_gen_text.cpp. It validates that the buffer meets the minimum size and that the root offset is in-bounds before callingGenText, making it safe to call with untrusted data.GenText/GenerateTextdeclarations inidl.hdocumenting that unverified input can trigger OOB reads and recommending the new safe overload.Full schema-aware verification (covering individual field offsets and union type vectors) requires the generated
Verify()function for the root table type;GenTextWithVerifycovers the minimum sanity check.Testing
Valid buffers pass the size and root-offset checks unchanged and behave identically to the existing
GenTextcall. The new overload returns an error string on buffers that are too small or have an out-of-bounds root pointer.