From 70b77de0d5aa8f70f87244961dc8fae2b1c9b928 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 9 Sep 2024 10:32:59 -0700 Subject: [PATCH 1/2] Fix a potential Ruby-upb use of uninitialized memory. Introduce a upb_MessageValue_Zero() function to use for the cases we do want a zero'd union (typically a zero MessageValue union is not needed) PiperOrigin-RevId: 672592274 --- ruby/ext/google/protobuf_c/defs.c | 2 +- upb/json/decode.c | 2 -- upb/message/value.h | 26 ++++++++++++++++++++++++++ upb/reflection/message.c | 3 +-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 7fd834acf617..825e22c65dd9 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -726,7 +726,7 @@ static VALUE FieldDescriptor__type(VALUE _self) { static VALUE FieldDescriptor_default(VALUE _self) { FieldDescriptor* self = ruby_to_FieldDescriptor(_self); const upb_FieldDef* f = self->fielddef; - upb_MessageValue default_val = {0}; + upb_MessageValue default_val = upb_MessageValue_Zero(); if (upb_FieldDef_IsSubMessage(f)) { return Qnil; } else if (!upb_FieldDef_IsRepeated(f)) { diff --git a/upb/json/decode.c b/upb/json/decode.c index 5659a017aac8..9275bcb1af50 100644 --- a/upb/json/decode.c +++ b/upb/json/decode.c @@ -712,7 +712,6 @@ static upb_MessageValue jsondec_int(jsondec* d, const upb_FieldDef* f) { /* Parse UINT32 or UINT64 value. */ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: { @@ -750,7 +749,6 @@ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { static upb_MessageValue jsondec_double(jsondec* d, const upb_FieldDef* f) { upb_StringView str; upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: diff --git a/upb/message/value.h b/upb/message/value.h index 8b15d1d3ad07..bb1dbbdcbd5c 100644 --- a/upb/message/value.h +++ b/upb/message/value.h @@ -12,9 +12,17 @@ #define UPB_MESSAGE_VALUE_H_ #include +#include #include "upb/base/string_view.h" +// Must be last. +#include "upb/port/def.inc" + +#ifdef __cplusplus +extern "C" { +#endif + typedef union { bool bool_val; float float_val; @@ -35,10 +43,28 @@ typedef union { uintptr_t tagged_msg_val; // upb_TaggedMessagePtr } upb_MessageValue; +UPB_API_INLINE upb_MessageValue upb_MessageValue_Zero(void) { + upb_MessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + typedef union { struct upb_Array* array; struct upb_Map* map; struct upb_Message* msg; } upb_MutableMessageValue; +UPB_API_INLINE upb_MutableMessageValue upb_MutableMessageValue_Zero(void) { + upb_MutableMessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + +#ifdef __cplusplus +} /* extern "C" */ +#endif + +#include "upb/port/undef.inc" + #endif /* UPB_MESSAGE_VALUE_H_ */ diff --git a/upb/reflection/message.c b/upb/reflection/message.c index f47a777bf46e..43c7578a87d9 100644 --- a/upb/reflection/message.c +++ b/upb/reflection/message.c @@ -138,8 +138,7 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, const upb_MiniTable* mt = upb_MessageDef_MiniTable(m); size_t i = *iter; size_t n = upb_MiniTable_FieldCount(mt); - upb_MessageValue zero; - memset(&zero, 0, sizeof(zero)); + upb_MessageValue zero = upb_MessageValue_Zero(); UPB_UNUSED(ext_pool); // Iterate over normal fields, returning the first one that is set. From 60e585c8f451cafcf6fd00a6575d7f2be09904d1 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 9 Sep 2024 14:26:52 -0400 Subject: [PATCH 2/2] Update staleness --- php/ext/google/protobuf/php-upb.c | 5 +---- php/ext/google/protobuf/php-upb.h | 24 ++++++++++++++++++++++++ ruby/ext/google/protobuf_c/ruby-upb.c | 5 +---- ruby/ext/google/protobuf_c/ruby-upb.h | 24 ++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/php/ext/google/protobuf/php-upb.c b/php/ext/google/protobuf/php-upb.c index 55bedc6043f4..964f53ce5709 100644 --- a/php/ext/google/protobuf/php-upb.c +++ b/php/ext/google/protobuf/php-upb.c @@ -3083,7 +3083,6 @@ static upb_MessageValue jsondec_int(jsondec* d, const upb_FieldDef* f) { /* Parse UINT32 or UINT64 value. */ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: { @@ -3121,7 +3120,6 @@ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { static upb_MessageValue jsondec_double(jsondec* d, const upb_FieldDef* f) { upb_StringView str; upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: @@ -15769,8 +15767,7 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, const upb_MiniTable* mt = upb_MessageDef_MiniTable(m); size_t i = *iter; size_t n = upb_MiniTable_FieldCount(mt); - upb_MessageValue zero; - memset(&zero, 0, sizeof(zero)); + upb_MessageValue zero = upb_MessageValue_Zero(); UPB_UNUSED(ext_pool); // Iterate over normal fields, returning the first one that is set. diff --git a/php/ext/google/protobuf/php-upb.h b/php/ext/google/protobuf/php-upb.h index 4ad9bbfd64bd..65417a0e1134 100644 --- a/php/ext/google/protobuf/php-upb.h +++ b/php/ext/google/protobuf/php-upb.h @@ -1046,7 +1046,14 @@ UPB_API_INLINE size_t upb_Array_Size(const struct upb_Array* arr) { #define UPB_MESSAGE_VALUE_H_ #include +#include + + +// Must be last. +#ifdef __cplusplus +extern "C" { +#endif typedef union { bool bool_val; @@ -1068,12 +1075,29 @@ typedef union { uintptr_t tagged_msg_val; // upb_TaggedMessagePtr } upb_MessageValue; +UPB_API_INLINE upb_MessageValue upb_MessageValue_Zero(void) { + upb_MessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + typedef union { struct upb_Array* array; struct upb_Map* map; struct upb_Message* msg; } upb_MutableMessageValue; +UPB_API_INLINE upb_MutableMessageValue upb_MutableMessageValue_Zero(void) { + upb_MutableMessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + +#ifdef __cplusplus +} /* extern "C" */ +#endif + + #endif /* UPB_MESSAGE_VALUE_H_ */ #ifndef UPB_MINI_TABLE_FIELD_H_ diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c index a8d241261493..98715cb8d2b8 100644 --- a/ruby/ext/google/protobuf_c/ruby-upb.c +++ b/ruby/ext/google/protobuf_c/ruby-upb.c @@ -2571,7 +2571,6 @@ static upb_MessageValue jsondec_int(jsondec* d, const upb_FieldDef* f) { /* Parse UINT32 or UINT64 value. */ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: { @@ -2609,7 +2608,6 @@ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) { static upb_MessageValue jsondec_double(jsondec* d, const upb_FieldDef* f) { upb_StringView str; upb_MessageValue val; - memset(&val, 0, sizeof(val)); switch (jsondec_peek(d)) { case JD_NUMBER: @@ -15257,8 +15255,7 @@ bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, const upb_MiniTable* mt = upb_MessageDef_MiniTable(m); size_t i = *iter; size_t n = upb_MiniTable_FieldCount(mt); - upb_MessageValue zero; - memset(&zero, 0, sizeof(zero)); + upb_MessageValue zero = upb_MessageValue_Zero(); UPB_UNUSED(ext_pool); // Iterate over normal fields, returning the first one that is set. diff --git a/ruby/ext/google/protobuf_c/ruby-upb.h b/ruby/ext/google/protobuf_c/ruby-upb.h index ab2f09e8f233..cf6ee92d2786 100755 --- a/ruby/ext/google/protobuf_c/ruby-upb.h +++ b/ruby/ext/google/protobuf_c/ruby-upb.h @@ -1048,7 +1048,14 @@ UPB_API_INLINE size_t upb_Array_Size(const struct upb_Array* arr) { #define UPB_MESSAGE_VALUE_H_ #include +#include + + +// Must be last. +#ifdef __cplusplus +extern "C" { +#endif typedef union { bool bool_val; @@ -1070,12 +1077,29 @@ typedef union { uintptr_t tagged_msg_val; // upb_TaggedMessagePtr } upb_MessageValue; +UPB_API_INLINE upb_MessageValue upb_MessageValue_Zero(void) { + upb_MessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + typedef union { struct upb_Array* array; struct upb_Map* map; struct upb_Message* msg; } upb_MutableMessageValue; +UPB_API_INLINE upb_MutableMessageValue upb_MutableMessageValue_Zero(void) { + upb_MutableMessageValue zero; + memset(&zero, 0, sizeof(zero)); + return zero; +} + +#ifdef __cplusplus +} /* extern "C" */ +#endif + + #endif /* UPB_MESSAGE_VALUE_H_ */ #ifndef UPB_MINI_TABLE_FIELD_H_