DBusMessage: Fix UB (misaligned access) in call to _dbus_header_set_field_basic()

The const void* 'value' pointer that is passed the address of a
uint32_t here eventually ends up in _dbus_marshal_write_basic(), which
casts it to a DBusBasicValue, a union type that has an alignment of
eight on 64-bit platforms and is therefore more-aligned than the
uint32.

The read of a value of a more-aligned type through a pointer to a less
-aligned type is undefined behaviour.

Fix by storing the uint32 in a DBusBasicValue and passing that instead.

Found by UBSan:

  dbus/dbus/dbus-marshal-basic.c:832:14: runtime error: member access within misaligned address 0x7fdb8dac3a04 for type 'const union DBusBasicValue', which requires 8 byte alignment
  0x7fdb8dac3a04: note: pointer points here
    4a 87 b5 71 01 00 00 00  40 7d 01 00 00 61 00 00  10 3b ac 8d db 7f 00 00  2c 2a 3e 94 db 7f 00 00
                ^
    #0 0x7fdb9444a2c3 in _dbus_marshal_write_basic dbus/dbus/dbus-marshal-basic.c:832
    #1 0x7fdb943d22fb in _dbus_type_writer_write_basic_no_typecode dbus/dbus/dbus-marshal-recursive.c:1605
    #2 0x7fdb943d64e9 in _dbus_type_writer_write_basic dbus/dbus/dbus-marshal-recursive.c:2327
    #3 0x7fdb943c52a6 in write_basic_field dbus/dbus/dbus-marshal-header.c:318
    #4 0x7fdb943c919e in _dbus_header_set_field_basic dbus/dbus/dbus-marshal-header.c:1321
    #5 0x7fdb943e1349 in dbus_message_set_reply_serial dbus/dbus/dbus-message.c:1173

Change-Id: I0149da4ebbead9b4b38c8c62af1ea892e24ec95e
Signed-off-by: Marc Mutz <marc@kdab.net>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98035

1 files changed, 62 insertions, 60 deletions

diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 29b85500..67665a1e 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -251,9 +251,9 @@ _dbus_message_byteswap (DBusMessage *message)
     return;
 
   _dbus_verbose ("Swapping message into compiler byte order\n");
-  
+
   get_const_signature (message, &type_str, &type_pos);
-  
+
   _dbus_marshal_byteswap (type_str, type_pos,
                           byte_order,
                           DBUS_COMPILER_BYTE_ORDER,
@@ -328,14 +328,14 @@ void _dbus_message_get_unix_fds(DBusMessage *message,
  * Sets the serial number of a message.
  * This can only be done once on a message.
  *
- * DBusConnection will automatically set the serial to an appropriate value 
- * when the message is sent; this function is only needed when encapsulating 
+ * DBusConnection will automatically set the serial to an appropriate value
+ * when the message is sent; this function is only needed when encapsulating
  * messages in another protocol, or otherwise bypassing DBusConnection.
  *
  * @param message the message
  * @param serial the serial
  */
-void 
+void
 dbus_message_set_serial (DBusMessage   *message,
                          dbus_uint32_t  serial)
 {
@@ -460,7 +460,7 @@ _dbus_message_remove_counter (DBusMessage  *message,
  * reference to a message in the outgoing queue and change it
  * underneath us. Messages are locked when they enter the outgoing
  * queue (dbus_connection_send_message()), and the library complains
- * if the message is modified while locked. This function may also 
+ * if the message is modified while locked. This function may also
  * called externally, for applications wrapping D-Bus in another protocol.
  *
  * @param message the message to lock.
@@ -679,7 +679,7 @@ dbus_message_get_cached (void)
   _dbus_assert (_dbus_atomic_get (&message->refcount) == 0);
 
   _dbus_assert (message->counters == NULL);
-  
+
   _DBUS_UNLOCK (message_cache);
 
   return message;
@@ -827,7 +827,7 @@ dbus_message_cache_or_finalize (DBusMessage *message)
   _dbus_assert (_dbus_atomic_get (&message->refcount) == 0);
 
   _DBUS_UNLOCK (message_cache);
-  
+
   if (!was_cached)
     dbus_message_finalize (message);
 }
@@ -1049,7 +1049,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
                   const char *s;
                   _dbus_type_reader_read_basic (&array,
                                                 (void *) &s);
-                  
+
                   str_array[j] = _dbus_strdup (s);
                   if (str_array[j] == NULL)
                     {
@@ -1057,9 +1057,9 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
                       _DBUS_SET_OOM (error);
                       goto out;
                     }
-                  
+
                   ++j;
-                  
+
                   if (!_dbus_type_reader_next (&array))
                     _dbus_assert (j == n_elements);
                 }
@@ -1200,7 +1200,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
  *
  * The D-Bus specification goes into some more detail about header fields and
  * message types.
- * 
+ *
  * @{
  */
 
@@ -1245,7 +1245,8 @@ dbus_bool_t
 dbus_message_set_reply_serial (DBusMessage   *message,
                                dbus_uint32_t  reply_serial)
 {
-  int type = DBUS_TYPE_UINT32;
+  DBusBasicValue value;
+  int type;
 
   _dbus_return_val_if_fail (message != NULL, FALSE);
   _dbus_return_val_if_fail (!message->locked, FALSE);
@@ -1253,18 +1254,19 @@ dbus_message_set_reply_serial (DBusMessage   *message,
 
   if (_dbus_message_is_gvariant (message))
     {
-      dbus_uint64_t reply_serial_uint64 = reply_serial;
       type = DBUS_TYPE_UINT64;
-      return _dbus_header_set_field_basic (&message->header,
-                                           DBUS_HEADER_FIELD_REPLY_SERIAL,
-                                           type,
-                                           &reply_serial_uint64);
+      value.u64 = reply_serial;
+    }
+  else
+    {
+      type = DBUS_TYPE_UINT64;
+      value.u32 = reply_serial;
     }
 
   return _dbus_header_set_field_basic (&message->header,
                                        DBUS_HEADER_FIELD_REPLY_SERIAL,
                                        type,
-                                       &reply_serial);
+                                       &value);
 }
 
 /**
@@ -1487,7 +1489,7 @@ dbus_message_new (int message_type)
  *
  * Destination, path, interface, and method name can't contain
  * any invalid characters (see the D-Bus specification).
- * 
+ *
  * @param destination name that the message should be sent to or #NULL
  * @param path object path the message should be sent to
  * @param iface interface to invoke method on, or #NULL
@@ -1558,7 +1560,7 @@ dbus_message_new_method_return (DBusMessage *method_call)
  *
  * Path, interface, and signal name must all be valid (the D-Bus
  * specification defines the syntax of these fields).
- * 
+ *
  * @param path the path to the object emitting the signal
  * @param iface the interface the signal is emitted from
  * @param name name of the signal
@@ -1659,7 +1661,7 @@ dbus_message_new_error (DBusMessage *reply_to,
  *
  * @todo add _DBUS_GNUC_PRINTF to this (requires moving _DBUS_GNUC_PRINTF to
  * public header, see DBUS_DEPRECATED for an example)
- * 
+ *
  * @param reply_to the original message
  * @param error_name the error name
  * @param error_format the error message format as with printf
@@ -1982,7 +1984,7 @@ dbus_message_append_args_valist (DBusMessage *message,
           char buf[2];
 
           element_type = va_arg (var_args, int);
-              
+
           buf[0] = element_type;
           buf[1] = '\0';
           if (!dbus_message_iter_open_container (&iter,
@@ -1999,7 +2001,7 @@ dbus_message_append_args_valist (DBusMessage *message,
 
               value = va_arg (var_args, const DBusBasicValue**);
               n_elements = va_arg (var_args, int);
-              
+
               if (!dbus_message_iter_append_fixed_array (&array,
                                                          element_type,
                                                          value,
@@ -2014,12 +2016,12 @@ dbus_message_append_args_valist (DBusMessage *message,
               const char **value;
               int n_elements;
               int i;
-              
+
               value_p = va_arg (var_args, const char***);
               n_elements = va_arg (var_args, int);
 
               value = *value_p;
-              
+
               i = 0;
               while (i < n_elements)
                 {
@@ -2096,7 +2098,7 @@ dbus_message_append_args_valist (DBusMessage *message,
  *
  * If more arguments than requested are present, the requested
  * arguments are returned and the extra arguments are ignored.
- * 
+ *
  * @todo support DBUS_TYPE_STRUCT and DBUS_TYPE_VARIANT and complex arrays
  *
  * @param message the message
@@ -2160,7 +2162,7 @@ _dbus_message_iter_init_common (DBusMessage         *message,
    * message, we need to get in the right byte order
    */
   ensure_byte_order (message);
-  
+
   real->message = message;
   real->changed_stamp = message->changed_stamp;
   real->iter_type = iter_type;
@@ -2175,7 +2177,7 @@ _dbus_message_iter_init_common (DBusMessage         *message,
  * Some types of argument can only be read with #DBusMessageIter
  * however.
  *
- * The easiest way to iterate is like this: 
+ * The easiest way to iterate is like this:
  * @code
  * dbus_message_iter_init (message, &iter);
  * while ((current_type = dbus_message_iter_get_arg_type (&iter)) != DBUS_TYPE_INVALID)
@@ -2184,7 +2186,7 @@ _dbus_message_iter_init_common (DBusMessage         *message,
  *
  * #DBusMessageIter contains no allocated memory; it need not be
  * freed, and can be copied by assignment or memcpy().
- * 
+ *
  * @param message the message
  * @param iter pointer to an iterator to initialize
  * @returns #FALSE if the message has no arguments
@@ -2346,7 +2348,7 @@ dbus_message_iter_recurse (DBusMessageIter  *iter,
  * the variant's value.
  *
  * The returned string must be freed with dbus_free().
- * 
+ *
  * @param iter the message iterator
  * @returns the contained signature, or NULL if out of memory
  */
@@ -2539,20 +2541,20 @@ dbus_message_iter_get_array_len (DBusMessageIter *iter)
  * The value argument should be the address of a location to store the
  * returned array. So for int32 it should be a "const dbus_int32_t**"
  * The returned value is by reference and should not be freed.
- * 
+ *
  * This function should only be used if dbus_type_is_fixed() returns
  * #TRUE for the element type.
  *
  * If an array's elements are not fixed in size, you have to recurse
  * into the array with dbus_message_iter_recurse() and read the
  * elements one by one.
- * 
+ *
  * Because the array is not copied, this function runs in constant
  * time and is fast; it's much preferred over walking the entire array
  * with an iterator. (However, you can always use
  * dbus_message_iter_recurse(), even for fixed-length types;
  * dbus_message_iter_get_fixed_array() is just an optimization.)
- * 
+ *
  * @param iter the iterator
  * @param value location to store the block
  * @param n_elements number of elements in the block
@@ -3057,7 +3059,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
                              contained_signature != NULL) ||
                             (type == DBUS_TYPE_ARRAY &&
                              contained_signature != NULL), FALSE);
-  
+
   /* this would fail if the contained_signature is a dict entry, since
    * dict entries are invalid signatures standalone (they must be in
    * an array)
@@ -3087,7 +3089,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
                                         type,
                                         NULL, 0,
                                         &real_sub->u.writer);
-    } 
+    }
 }
 
 
@@ -3166,7 +3168,7 @@ dbus_message_iter_abandon_container (DBusMessageIter *iter,
  * required to reply.
  *
  * On the protocol level this toggles #DBUS_HEADER_FLAG_NO_REPLY_EXPECTED
- * 
+ *
  * @param message the message
  * @param no_reply #TRUE if no reply is desired
  */
@@ -3208,7 +3210,7 @@ dbus_message_get_no_reply (DBusMessage *message)
  * The flag is set to #TRUE by default, i.e. auto starting is the default.
  *
  * On the protocol level this toggles #DBUS_HEADER_FLAG_NO_AUTO_START
- * 
+ *
  * @param message the message
  * @param auto_start #TRUE if auto-starting is desired
  */
@@ -3278,7 +3280,7 @@ dbus_message_set_path (DBusMessage   *message,
  *
  * The returned string becomes invalid if the message is
  * modified, since it points into the wire-marshaled message data.
- * 
+ *
  * @param message the message
  * @returns the path (should not be freed) or #NULL
  */
@@ -3312,7 +3314,7 @@ dbus_message_has_path (DBusMessage   *message,
 {
   const char *msg_path;
   msg_path = dbus_message_get_path (message);
-  
+
   if (msg_path == NULL)
     {
       if (path == NULL)
@@ -3323,7 +3325,7 @@ dbus_message_has_path (DBusMessage   *message,
 
   if (path == NULL)
     return FALSE;
-   
+
   if (strcmp (msg_path, path) == 0)
     return TRUE;
 
@@ -3342,7 +3344,7 @@ dbus_message_has_path (DBusMessage   *message,
  * and the path "/" becomes { NULL }.
  *
  * See also dbus_message_get_path().
- * 
+ *
  * @todo this could be optimized by using the len from the message
  * instead of calling strlen() again
  *
@@ -3379,7 +3381,7 @@ dbus_message_get_path_decomposed (DBusMessage   *message,
  *
  * The interface name must contain only valid characters as defined
  * in the D-Bus specification.
- * 
+ *
  * @param message the message
  * @param iface the interface or #NULL to unset
  * @returns #FALSE if not enough memory
@@ -3441,7 +3443,7 @@ dbus_message_has_interface (DBusMessage   *message,
 {
   const char *msg_interface;
   msg_interface = dbus_message_get_interface (message);
-   
+
   if (msg_interface == NULL)
     {
       if (iface == NULL)
@@ -3452,7 +3454,7 @@ dbus_message_has_interface (DBusMessage   *message,
 
   if (iface == NULL)
     return FALSE;
-     
+
   if (strcmp (msg_interface, iface) == 0)
     return TRUE;
 
@@ -3495,7 +3497,7 @@ dbus_message_set_member (DBusMessage  *message,
  *
  * The returned string becomes invalid if the message is
  * modified, since it points into the wire-marshaled message data.
- * 
+ *
  * @param message the message
  * @returns the member name (should not be freed) or #NULL
  */
@@ -3527,7 +3529,7 @@ dbus_message_has_member (DBusMessage   *message,
 {
   const char *msg_member;
   msg_member = dbus_message_get_member (message);
- 
+
   if (msg_member == NULL)
     {
       if (member == NULL)
@@ -3538,7 +3540,7 @@ dbus_message_has_member (DBusMessage   *message,
 
   if (member == NULL)
     return FALSE;
-    
+
   if (strcmp (msg_member, member) == 0)
     return TRUE;
 
@@ -3579,7 +3581,7 @@ dbus_message_set_error_name (DBusMessage  *message,
  *
  * The returned string becomes invalid if the message is
  * modified, since it points into the wire-marshaled message data.
- * 
+ *
  * @param message the message
  * @returns the error name (should not be freed) or #NULL
  */
@@ -3606,7 +3608,7 @@ dbus_message_get_error_name (DBusMessage *message)
  *
  * The destination name must contain only valid characters as defined
  * in the D-Bus specification.
- * 
+ *
  * @param message the message
  * @param destination the destination name or #NULL to unset
  * @returns #FALSE if not enough memory
@@ -3689,7 +3691,7 @@ dbus_message_set_sender (DBusMessage  *message,
  * Note, the returned sender is always the unique bus name.
  * Connections may own multiple other bus names, but those
  * are not found in the sender field.
- * 
+ *
  * The returned string becomes invalid if the message is
  * modified, since it points into the wire-marshaled message data.
  *
@@ -4060,7 +4062,7 @@ _dbus_message_loader_new (void)
   loader = dbus_new0 (DBusMessageLoader, 1);
   if (loader == NULL)
     return NULL;
-  
+
   loader->refcount = 1;
 
   loader->corrupted = FALSE;
@@ -4302,7 +4304,7 @@ load_message (DBusMessageLoader *loader,
   dbus_uint32_t n_unix_fds = 0;
 
   mode = DBUS_VALIDATION_MODE_DATA_IS_UNTRUSTED;
-  
+
   oom = FALSE;
 
 #if 0
@@ -4379,7 +4381,7 @@ load_message (DBusMessageLoader *loader,
 
           loader->corrupted = TRUE;
           loader->corruption_reason = validity;
-          
+
           goto failed;
         }
     }
@@ -4483,7 +4485,7 @@ load_message (DBusMessageLoader *loader,
 
   /* does nothing if the message isn't in the list */
   _dbus_list_remove_last (&loader->messages, message);
-  
+
   if (oom)
     _dbus_assert (!loader->corrupted);
   else
@@ -5151,7 +5153,7 @@ dbus_message_marshal (DBusMessage  *msg,
   _dbus_return_val_if_fail (msg != NULL, FALSE);
   _dbus_return_val_if_fail (marshalled_data_p != NULL, FALSE);
   _dbus_return_val_if_fail (len_p != NULL, FALSE);
-  
+
   if (!_dbus_string_init (&tmp))
     return FALSE;
 
@@ -5261,10 +5263,10 @@ dbus_message_demarshal (const char *str,
  * @param buf data to be marshalled
  * @param len the length of @p buf
  * @returns -1 if there was no valid data to be demarshalled, 0 if there wasn't enough data to determine how much should be demarshalled. Otherwise returns the number of bytes to be demarshalled
- * 
+ *
  */
-int 
-dbus_message_demarshal_bytes_needed(const char *buf, 
+int
+dbus_message_demarshal_bytes_needed(const char *buf,
                                     int         len)
 {
   DBusString str;
@@ -5279,7 +5281,7 @@ dbus_message_demarshal_bytes_needed(const char *buf,
   if (len > DBUS_MAXIMUM_MESSAGE_LENGTH)
     len = DBUS_MAXIMUM_MESSAGE_LENGTH;
   _dbus_string_init_const_len (&str, buf, len);
-  
+
   validity = DBUS_VALID;
   have_message
     = _dbus_header_have_message_untrusted(DBUS_MAXIMUM_MESSAGE_LENGTH,