Merge pull request #18191 from protocolbuffers/cp-ruby-upb

Fix a potential Ruby-upb use of uninitialized memory.
protocolbuffers · Sep 9, 2024 · d70f077 · d70f077
2 parents 5b4b3af + 60e585c
commit d70f077
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 13 deletions.
diff --git 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
@@ -1046,7 +1046,14 @@ UPB_API_INLINE size_t upb_Array_Size(const struct upb_Array* arr) {
 #define UPB_MESSAGE_VALUE_H_
 
 #include <stdint.h>
+#include <string.h>
+
+
+// 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/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/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
@@ -1048,7 +1048,14 @@ UPB_API_INLINE size_t upb_Array_Size(const struct upb_Array* arr) {
 #define UPB_MESSAGE_VALUE_H_
 
 #include <stdint.h>
+#include <string.h>
+
+
+// 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_

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.