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
protocolbuffers · Sep 9, 2024 · 4d75c82 · 4d75c82
1 parent 44e9777
commit 4d75c82
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 5 deletions.
diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c
@@ -725,7 +725,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
@@ -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
@@ -12,9 +12,17 @@
 #define UPB_MESSAGE_VALUE_H_
 
 #include <stdint.h>
+#include <string.h>
 
 #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
@@ -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.