Add bounds check for root offset in AddFlatBuffer#8982
Add bounds check for root offset in AddFlatBuffer#8982jtdavis777 merged 2 commits intogoogle:masterfrom
Conversation
jtdavis777
left a comment
There was a problem hiding this comment.
This looks like a great change to have in place, my only comments are that flatbuffers is intended to be a very fast, low level library. There are many places in the code base that make the intentional trade off to favor speed over protection, and this seems like one of those places. This function is intending to be called from within someone's code where they have constructed a valid flatbuffers object to add in here -- what is the actual level of risk to leaving this function as is?
|
I agree FlatBuffers often favors speed on hot paths. In this case, the added checks are intentionally minimal and are before work that already does buffer alignment and a full insert/copy, so the runtime impact should be negligible relative to existing cost. The motivation is to avoid undefined behavior on malformed input in release builds (where assertions may be disabled): previously we could read/use an invalid root offset for pointer arithmetic. With this change, invalid input returns nullptr; valid input behavior is unchanged. |
AddFlatBuffer reads the root offset from the input buffer and uses it
for pointer arithmetic without validating that it lies within the buffer bounds.
This change adds a minimal bounds check before using the offset to avoid
undefined behavior when malformed data is passed.
No behavior change for valid inputs.