protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

upb: Null pointer dereference adding syntax3 file descriptor to upb_DefPool

Open emcfarlane opened this issue 1 year ago • 1 comments

What version of protobuf and what language are you using? Version: main@a46d82f (checked out 26th Jan 2024) Language: C (upb)

What operating system (Linux, Windows, ...) and version? MacOS 14.3 (Darwin arm64).

What runtime / compiler are you using (e.g., python version or gcc version) I am building upb and using the library outside of bazel and the usual build systems which may affect the issue.

What did you do? Steps to reproduce the behavior:

  1. Generate upb c library with the following options:
--upb_out=. --upb_minitable_out=. --upbdefs_out=.

Proto file is a simple example message:

syntax = "proto3"; package example; message Message { string hello = 1; }
  1. Call _upb_DefPool_LoadDefInit with an allocated upb_DefPool and the generated example_proto_upbdefinit reference.
  2. Crashes on exception.

What did you expect to see

Successful return.

What did you see instead?

Exception on null pointer dereference. Call stack:

google_protobuf_FieldOptions_has_packed (./protobuf/google/protobuf/descriptor.upb.h:3973)
_upb_FieldDef_InferLegacyFeatures (./protobuf/upb/reflection/field_def.c:560)
_upb_FieldDef_Create (./protobuf/upb/reflection/field_def.c:612)
_upb_FieldDef_CreateNotExt (./protobuf/upb/reflection/field_def.c:745)
_upb_FieldDefs_New (./protobuf/upb/reflection/field_def.c:792)
create_msgdef (./protobuf/upb/reflection/message_def.c:701)
_upb_MessageDefs_New (./protobuf/upb/reflection/message_def.c:761)
_upb_FileDef_Create (./protobuf/upb/reflection/file_def.c:394)
upb_DefBuilder_AddFileToPool (./protobuf/upb/reflection/def_pool.c:362)
_upb_DefPool_AddFile (./protobuf/upb/reflection/def_pool.c:405)
_upb_DefPool_LoadDefInitEx (./protobuf/upb/reflection/def_pool.c:451)
_upb_DefPool_LoadDefInit (./protobuf/upb/reflection/def_pool.c:521)

Opening with a debugger I can see the field descriptor is correctly initialised before line 610 in field_def.c. Then when the message is cleared f->opts is set to null which is then passed to _upb_FieldDef_InferLegacyFeatures on line 612 causing the null pointer dereference.

https://github.com/protocolbuffers/protobuf/blob/6c58b9a552577b2253057764e9917248c49d4384/upb/reflection/field_def.c#L610-L613

Removing lines 610 and 611 fixes the issue.

emcfarlane avatar Feb 20 '24 12:02 emcfarlane

Would not using bazel cause this, @ericsalo ?

honglooker avatar Feb 20 '24 18:02 honglooker

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar May 21 '24 10:05 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Jun 04 '24 10:06 github-actions[bot]