Reject [weak = true] on non-message fields to prevent SEGV#27169
Open
jortles wants to merge 1 commit intoprotocolbuffers:mainfrom
Open
Reject [weak = true] on non-message fields to prevent SEGV#27169jortles wants to merge 1 commit intoprotocolbuffers:mainfrom
jortles wants to merge 1 commit intoprotocolbuffers:mainfrom
Conversation
…Message A crafted FileDescriptorProto with weak=true on a scalar field (e.g., TYPE_DOUBLE) causes a SEGV when the resulting DynamicMessage is used with ParseFromArray. The crash occurs because weak fields are assigned HasbitMode::kNoHasbit, which produces incorrect field offsets in the lazily-generated TcParseTable. TcParser then writes field data at the wrong offset, corrupting the DynamicMessage's internal type_info_ pointer, leading to a wild pointer dereference in GetMetadata(). The [weak] option is only meaningful for message-type fields (for cross-file dependency management). Add validation in ValidateOptions to reject [weak = true] on non-message fields, matching the existing pattern for [lazy = true].
|
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. |
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
A crafted
FileDescriptorProtowithweak=trueon a scalar field (e.g.,TYPE_DOUBLE) causes a SEGV (wild pointer dereference) when the resultingDynamicMessageis used withParseFromArray. This is a DoS vulnerability affecting any application that builds descriptors from untrusted input withAllowUnknownDependencies()(gRPC server reflection, schema registries, protoc, dynamic config systems).Root cause
weak=trueon a non-message field causesGetFieldHasbitModeWithoutProfile()to returnkNoHasbit(descriptor.cc:10751)DynamicMessage::ParseFromArray()lazily generates theTcParseTableviaReflection::CreateTcParseTable(), thekNoHasbithasbit index produces incorrect field offsets in the table entriesTcParserwrites the parsed field value at the wrong offset, corruptingDynamicMessage's internaltype_info_pointerGetMetadata()/GetDescriptor()/IsInitialized()dereference the corrupted pointer → SEGVCrash site
Fix
Add validation in
ValidateOptions()to reject[weak = true]on non-message fields, following the identical pattern already used for[lazy = true](line 8581). The[weak]option is only meaningful for message-type fields (cross-file dependency management) and has no valid use on scalar, enum, or group fields.Reproduction
Found by AFL++ fuzzing with custom harness targeting
BuildFile+DynamicMessageFactorypipeline.