From d22c762221933bc4fb4a27ec3bbbe48617ced917 Mon Sep 17 00:00:00 2001
From: Philip Withnall <withnall@endlessm.com>
Date: Thu, 27 Feb 2020 11:20:26 +0000
Subject: [PATCH] garray: Fix copying an array with reserved elements
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Spotted by Mohammed Sadiq. `g_array_copy()` was doing a `memcpy()` of
the data from the old array to the new one, based on the reserved
elements in the old array (`array->alloc`). However, the new array was
allocated based on the *assigned* elements in the old array
(`array->len`).

So if the old array had fewer assigned elements than allocated elements,
`memcpy()` would fall off the end of the newly allocated data block.
This was particularly obvious when the old array had no assigned
elements, as the new array’s data pointer would be `NULL`.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #2049
---
 glib/garray.c           |  7 +++++--
 glib/tests/array-test.c | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/glib/garray.c b/glib/garray.c
index d6b3bdafa..4458c8eea 100644
--- a/glib/garray.c
+++ b/glib/garray.c
@@ -1242,9 +1242,12 @@ g_array_copy (GArray *array)
 
   new_rarray =
     (GRealArray *) g_array_sized_new (rarray->zero_terminated, rarray->clear,
-                                      rarray->elt_size, rarray->len);
+                                      rarray->elt_size, rarray->alloc / rarray->elt_size);
   new_rarray->len = rarray->len;
-  memcpy (new_rarray->data, rarray->data, rarray->alloc);
+  if (rarray->len > 0)
+    memcpy (new_rarray->data, rarray->data, rarray->len * rarray->elt_size);
+
+  g_array_zero_terminate (new_rarray);
 
   return (GArray *) new_rarray;
 }
diff --git a/glib/tests/array-test.c b/glib/tests/array-test.c
index b90b81f73..bdd6a2cb9 100644
--- a/glib/tests/array-test.c
+++ b/glib/tests/array-test.c
@@ -811,6 +811,32 @@ test_array_binary_search (void)
   g_array_free (garray, TRUE);
 }
 
+static void
+test_array_copy_sized (void)
+{
+  GArray *array1 = NULL, *array2 = NULL, *array3 = NULL;
+  int val = 5;
+
+  g_test_summary ("Test that copying a newly-allocated sized array works.");
+
+  array1 = g_array_sized_new (FALSE, FALSE, sizeof (int), 1);
+  array2 = g_array_copy (array1);
+
+  g_assert_cmpuint (array2->len, ==, array1->len);
+
+  g_array_append_val (array1, val);
+  array3 = g_array_copy (array1);
+
+  g_assert_cmpuint (array3->len, ==, array1->len);
+  g_assert_cmpuint (g_array_index (array3, int, 0), ==, g_array_index (array1, int, 0));
+  g_assert_cmpuint (array3->len, ==, 1);
+  g_assert_cmpuint (g_array_index (array3, int, 0), ==, val);
+
+  g_array_unref (array3);
+  g_array_unref (array2);
+  g_array_unref (array1);
+}
+
 /* Check g_ptr_array_steal() function */
 static void
 pointer_array_steal (void)
@@ -1955,6 +1981,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/array/steal", array_steal);
   g_test_add_func ("/array/clear-func", array_clear_func);
   g_test_add_func ("/array/binary-search", test_array_binary_search);
+  g_test_add_func ("/array/copy-sized", test_array_copy_sized);
 
   for (i = 0; i < G_N_ELEMENTS (array_configurations); i++)
     {