Verify untrusted embedded flatbuffer/flexbuffer before parsing in detection_postprocessing_graph - #6336
Open
hdk10 wants to merge 1 commit into
Open
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. |
Author
|
@googlebot I signed it! |
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
detection_postprocessing_graph.ccparses two untrusted, model-embedded buffers with no verification,plus one missing null-check:
Nested
ObjectDetectorOptionsflatbuffer.ConfigureOutModelNmsTensorsToDetectionsCalculatorand
ConfigureSsdAnchorsCalculatorcallGetObjectDetectorOptions(custom_metadata->data()->data())(a bare
GetRoot, noVerifier) on the attacker-controlledDETECTOR_METADATACustomMetadatablob. The outer metadata verifier only checks the byte-vector length, not that the bytes form a
valid nested flatbuffer, so a malformed blob leads to an out-of-bounds read / wild-pointer walk.
flexbuffers::GetRootoncustom_options.GetMaxClassesPerDetectioncallsflexbuffers::GetRoot(custom_options->Data(), custom_options->size())with noflexbuffers::VerifyBuffer.GetRootreads the trailing bytes to seed parsing, so a 0/1-bytecustom_optionsunderflows before the buffer start.Null
custom_code(). The op-code search dereferencescustom_code()->str()without checkingcustom_code() != nullptr(an optional field), crashing on a CUSTOM op-code lacking it.Impact
Out-of-bounds read / null dereference (crash / potential info disclosure) during
ObjectDetector::Create()on a crafted model, before inference.Change
Verify the
ObjectDetectorOptionsbuffer withVerifyObjectDetectorOptionsBufferbefore parsing (bothsites); verify the flexbuffer with
flexbuffers::VerifyBufferbeforeGetRoot; null-checkcustom_code(). No behavior change for valid models.Notes
Reported through the Google OSS VRP (issue 546472889). Verified with AddressSanitizer PoCs against the
GetObjectDetectorOptionsandflexbuffers::GetRootsinks. Analysis was AI-assisted andhuman-reviewed; I could not run the full Bazel build locally, so please run CI.