[glib] Avoid calling Standard C string/array functions with NULL arguments

[Simon McVittie <smcv src gnome org>]

commit e5ed410c8c0fe823883b65b293fb2d9c9d12673a
Author: Simon McVittie <smcv debian org>
Date:   Fri Dec 2 10:03:16 2016 +0000

    Avoid calling Standard C string/array functions with NULL arguments
    
    glibc string.h declares memcpy() with attribute(nonnull(1,2)), causing
    calls with NULL arguments to be treated as undefined behaviour.
    This is consistent with ISO C99 and C11, which state that passing 0
    to string functions as an array length does not remove the requirement
    that the pointer to the array is a valid pointer.
    gcc -fsanitize=undefined catches this while running OSTree's test suite.
    
    Similarly, running the GLib test suite reports similar issues for
    qsort(), memmove(), memcmp().
    
    Signed-off-by: Simon McVittie <smcv debian org>
    Bug: https://bugzilla.gnome.org/show_bug.cgi?id=775510
    Reviewed-by: Colin Walters

 gio/gconverterinputstream.c  |   12 ++++++++----
 gio/gconverteroutputstream.c |    8 +++++---
 gio/gdesktopappinfo.c        |   13 ++++++++-----
 gio/gunixsocketaddress.c     |    4 +++-
 gio/gvdb/gvdb-builder.c      |    3 ++-
 glib/garray.c                |    9 +++++++++
 glib/goption.c               |    5 ++++-
 glib/gstrfuncs.c             |    2 +-
 glib/gtestutils.h            |    2 +-
 gobject/gtype.c              |    3 ++-
 10 files changed, 43 insertions(+), 18 deletions(-)
---
diff --git a/gio/gconverterinputstream.c b/gio/gconverterinputstream.c
index d23b8f2..5fb26cb 100644
--- a/gio/gconverterinputstream.c
+++ b/gio/gconverterinputstream.c
@@ -262,7 +262,9 @@ buffer_read (Buffer *buffer,
             char *dest,
             gsize count)
 {
-  memcpy (dest, buffer->data + buffer->start, count);
+  if (count != 0)
+    memcpy (dest, buffer->data + buffer->start, count);
+
   buffer_consumed (buffer, count);
 }
 
@@ -293,9 +295,11 @@ grow_buffer (Buffer *buffer)
   data = g_malloc (size);
   in_buffer = buffer_data_size (buffer);
 
-  memcpy (data,
-         buffer->data + buffer->start,
-         in_buffer);
+  if (in_buffer != 0)
+    memcpy (data,
+            buffer->data + buffer->start,
+            in_buffer);
+
   g_free (buffer->data);
   buffer->data = data;
   buffer->end -= buffer->start;
diff --git a/gio/gconverteroutputstream.c b/gio/gconverteroutputstream.c
index 46616b1..eef277e 100644
--- a/gio/gconverteroutputstream.c
+++ b/gio/gconverteroutputstream.c
@@ -300,9 +300,11 @@ grow_buffer (Buffer *buffer)
   data = g_malloc (size);
   in_buffer = buffer_data_size (buffer);
 
-  memcpy (data,
-         buffer->data + buffer->start,
-         in_buffer);
+  if (in_buffer != 0)
+    memcpy (data,
+            buffer->data + buffer->start,
+            in_buffer);
+
   g_free (buffer->data);
   buffer->data = data;
   buffer->end -= buffer->start;
diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
index 4668ea7..81dff27 100644
--- a/gio/gdesktopappinfo.c
+++ b/gio/gdesktopappinfo.c
@@ -506,7 +506,8 @@ add_token_result (const gchar *app_name,
 static void
 merge_token_results (gboolean first)
 {
-  qsort (static_token_results, static_token_results_size, sizeof (struct search_result), compare_results);
+  if (static_token_results_size != 0)
+    qsort (static_token_results, static_token_results_size, sizeof (struct search_result), compare_results);
 
   /* If this is the first token then we are basically merging a list with
    * itself -- we only perform de-duplication.
@@ -606,7 +607,8 @@ reset_total_search_results (void)
 static void
 sort_total_search_results (void)
 {
-  qsort (static_total_results, static_total_results_size, sizeof (struct search_result), compare_categories);
+  if (static_total_results_size != 0)
+    qsort (static_total_results, static_total_results_size, sizeof (struct search_result), 
compare_categories);
 }
 
 static void
@@ -620,9 +622,10 @@ merge_directory_results (void)
       static_total_results = g_renew (struct search_result, static_total_results, 
static_total_results_allocated);
     }
 
-  memcpy (static_total_results + static_total_results_size,
-          static_search_results,
-          static_search_results_size * sizeof (struct search_result));
+  if (static_total_results + static_total_results_size != 0)
+    memcpy (static_total_results + static_total_results_size,
+            static_search_results,
+            static_search_results_size * sizeof (struct search_result));
 
   static_total_results_size += static_search_results_size;
 
diff --git a/gio/gunixsocketaddress.c b/gio/gunixsocketaddress.c
index f4fc077..3b2a1c4 100644
--- a/gio/gunixsocketaddress.c
+++ b/gio/gunixsocketaddress.c
@@ -116,7 +116,9 @@ g_unix_socket_address_set_property (GObject      *object,
          /* Clip to fit in UNIX_PATH_MAX with zero termination or first byte */
          len = MIN (array->len, UNIX_PATH_MAX-1);
 
-         memcpy (address->priv->path, array->data, len);
+         if (len != 0)
+           memcpy (address->priv->path, array->data, len);
+
          address->priv->path[len] = 0; /* Ensure null-terminated */
          address->priv->path_len = len;
        }
diff --git a/gio/gvdb/gvdb-builder.c b/gio/gvdb/gvdb-builder.c
index 48fe3e4..43ad068 100644
--- a/gio/gvdb/gvdb-builder.c
+++ b/gio/gvdb/gvdb-builder.c
@@ -291,7 +291,8 @@ file_builder_add_string (FileBuilder *fb,
   chunk->offset = fb->offset;
   chunk->size = length;
   chunk->data = g_malloc (length);
-  memcpy (chunk->data, string, length);
+  if (length != 0)
+    memcpy (chunk->data, string, length);
 
   *start = guint32_to_le (fb->offset);
   *size = guint16_to_le (length);
diff --git a/glib/garray.c b/glib/garray.c
index 0b4bf04..ac7e580 100644
--- a/glib/garray.c
+++ b/glib/garray.c
@@ -415,6 +415,9 @@ g_array_append_vals (GArray       *farray,
 
   g_return_val_if_fail (array, NULL);
 
+  if (len == 0)
+    return farray;
+
   g_array_maybe_expand (array, len);
 
   memcpy (g_array_elt_pos (array, array->len), data, 
@@ -468,6 +471,9 @@ g_array_prepend_vals (GArray        *farray,
 
   g_return_val_if_fail (array, NULL);
 
+  if (len == 0)
+    return farray;
+
   g_array_maybe_expand (array, len);
 
   memmove (g_array_elt_pos (array, len), g_array_elt_pos (array, 0),
@@ -517,6 +523,9 @@ g_array_insert_vals (GArray        *farray,
 
   g_return_val_if_fail (array, NULL);
 
+  if (len == 0)
+    return farray;
+
   g_array_maybe_expand (array, len);
 
   memmove (g_array_elt_pos (array, len + index_),
diff --git a/glib/goption.c b/glib/goption.c
index e7d35bc..7a21d1c 100644
--- a/glib/goption.c
+++ b/glib/goption.c
@@ -2361,7 +2361,10 @@ g_option_group_add_entries (GOptionGroup       *group,
 
   group->entries = g_renew (GOptionEntry, group->entries, group->n_entries + n_entries);
 
-  memcpy (group->entries + group->n_entries, entries, sizeof (GOptionEntry) * n_entries);
+  /* group->entries could be NULL in the trivial case where we add no
+   * entries to no entries */
+  if (n_entries != 0)
+    memcpy (group->entries + group->n_entries, entries, sizeof (GOptionEntry) * n_entries);
 
   for (i = group->n_entries; i < group->n_entries + n_entries; i++)
     {
diff --git a/glib/gstrfuncs.c b/glib/gstrfuncs.c
index f8dc0b1..eb0f1de 100644
--- a/glib/gstrfuncs.c
+++ b/glib/gstrfuncs.c
@@ -386,7 +386,7 @@ g_memdup (gconstpointer mem,
 {
   gpointer new_mem;
 
-  if (mem)
+  if (mem && byte_size != 0)
     {
       new_mem = g_malloc (byte_size);
       memcpy (new_mem, mem, byte_size);
diff --git a/glib/gtestutils.h b/glib/gtestutils.h
index d788eb9..99e237d 100644
--- a/glib/gtestutils.h
+++ b/glib/gtestutils.h
@@ -74,7 +74,7 @@ typedef void (*GTestFixtureFunc) (gpointer      fixture,
                                              if (__l1 != __l2) \
                                                g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, 
G_STRFUNC, \
                                                                            #l1 " (len(" #m1 ")) == " #l2 " 
(len(" #m2 "))", __l1, "==", __l2, 'i'); \
-                                             else if (memcmp (__m1, __m2, __l1) != 0) \
+                                             else if (__l1 != 0 && memcmp (__m1, __m2, __l1) != 0) \
                                                g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, 
G_STRFUNC, \
                                                                     "assertion failed (" #m1 " == " #m2 
")"); \
                                         } G_STMT_END
diff --git a/gobject/gtype.c b/gobject/gtype.c
index 48290ae..f381a78 100644
--- a/gobject/gtype.c
+++ b/gobject/gtype.c
@@ -3550,7 +3550,8 @@ g_type_children (GType  type,
       
       G_READ_LOCK (&type_rw_lock);     /* ->children is relocatable */
       children = g_new (GType, node->n_children + 1);
-      memcpy (children, node->children, sizeof (GType) * node->n_children);
+      if (node->n_children != 0)
+        memcpy (children, node->children, sizeof (GType) * node->n_children);
       children[node->n_children] = 0;
       
       if (n_children)