From 89b522ed31837cb2ac107a8961fbb0f2c7fc7ccb Mon Sep 17 00:00:00 2001

From: Krzesimir Nowak <qdlacz@gmail.com>

Date: Wed, 10 Feb 2021 23:51:07 +0100

Subject: [PATCH] gbytearray: Do not accept too large byte arrays

GByteArray uses guint for storing the length of the byte array, but it

also has a constructor (g_byte_array_new_take) that takes length as a

gsize. gsize may be larger than guint (64 bits for gsize vs 32 bits

for guint). It is possible to call the function with a value greater

than G_MAXUINT, which will result in silent length truncation. This

may happen as a result of unreffing GBytes into GByteArray, so rather

be loud about it.

(Test case tweaked by Philip Withnall.)

---

 glib/garray.c      |  6 ++++++

 glib/gbytes.c      |  4 ++++

 glib/tests/bytes.c | 37 +++++++++++++++++++++++++++++++++++--

 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/glib/garray.c b/glib/garray.c

index aa3c04707..271d85ad8 100644

--- a/glib/garray.c

+++ b/glib/garray.c

@@ -1666,6 +1666,10 @@ g_byte_array_new (void)

  * Create byte array containing the data. The data will be owned by the array

  * and will be freed with g_free(), i.e. it could be allocated using g_strdup().

  *

+ * Do not use it if @len is greater than %G_MAXUINT. #GByteArray

+ * stores the length of its data in #guint, which may be shorter than

+ * #gsize.

+ *

  * Since: 2.32

  *

  * Returns: (transfer full): a new #GByteArray

@@ -1677,6 +1681,8 @@ g_byte_array_new_take (guint8 *data,

   GByteArray *array;

   GRealArray *real;

+  g_return_val_if_fail (len <= G_MAXUINT, NULL);

+

   array = g_byte_array_new ();

   real = (GRealArray *)array;

   g_assert (real->data == NULL);

diff --git a/glib/gbytes.c b/glib/gbytes.c

index 5141170d7..635b79535 100644

--- a/glib/gbytes.c

+++ b/glib/gbytes.c

@@ -512,6 +512,10 @@ g_bytes_unref_to_data (GBytes *bytes,

  * g_bytes_new(), g_bytes_new_take() or g_byte_array_free_to_bytes(). In all

  * other cases the data is copied.

  *

+ * Do not use it if @bytes contains more than %G_MAXUINT

+ * bytes. #GByteArray stores the length of its data in #guint, which

+ * may be shorter than #gsize, that @bytes is using.

+ *

  * Returns: (transfer full): a new mutable #GByteArray containing the same byte data

  *

  * Since: 2.32

diff --git a/glib/tests/bytes.c b/glib/tests/bytes.c

index 5ea5c2b35..42281307b 100644

--- a/glib/tests/bytes.c

+++ b/glib/tests/bytes.c

@@ -10,12 +10,12 @@

  */

 #undef G_DISABLE_ASSERT

-#undef G_LOG_DOMAIN

 #include <stdio.h>

 #include <stdlib.h>

 #include <string.h>

 #include "glib.h"

+#include "glib/gstrfuncsprivate.h"

 /* Keep in sync with glib/gbytes.c */

 struct _GBytes

@@ -333,6 +333,38 @@ test_to_array_transferred (void)

   g_byte_array_unref (array);

 }

+static void

+test_to_array_transferred_oversize (void)

+{

+  g_test_message ("g_bytes_unref_to_array() can only take GBytes up to "

+                  "G_MAXUINT in length; test that longer ones are rejected");

+

+  if (sizeof (guint) >= sizeof (gsize))

+    {

+      g_test_skip ("Skipping test as guint is not smaller than gsize");

+    }

+  else if (g_test_undefined ())

+    {

+      GByteArray *array = NULL;

+      GBytes *bytes = NULL;

+      gpointer data = g_memdup2 (NYAN, N_NYAN);

+      gsize len = ((gsize) G_MAXUINT) + 1;

+

+      bytes = g_bytes_new_take (data, len);

+      g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,

+                             "g_byte_array_new_take: assertion 'len <= G_MAXUINT' failed");

+      array = g_bytes_unref_to_array (g_steal_pointer (&bytes));

+      g_test_assert_expected_messages ();

+      g_assert_null (array);

+

+      g_free (data);

+    }

+  else

+    {

+      g_test_skip ("Skipping test as testing undefined behaviour is disabled");

+    }

+}

+

 static void

 test_to_array_two_refs (void)

 {

@@ -407,7 +439,8 @@ main (int argc, char *argv[])

   g_test_add_func ("/bytes/to-data/transfered", test_to_data_transferred);

   g_test_add_func ("/bytes/to-data/two-refs", test_to_data_two_refs);

   g_test_add_func ("/bytes/to-data/non-malloc", test_to_data_non_malloc);

-  g_test_add_func ("/bytes/to-array/transfered", test_to_array_transferred);

+  g_test_add_func ("/bytes/to-array/transferred", test_to_array_transferred);

+  g_test_add_func ("/bytes/to-array/transferred-oversize", test_to_array_transferred_oversize);

   g_test_add_func ("/bytes/to-array/two-refs", test_to_array_two_refs);

   g_test_add_func ("/bytes/to-array/non-malloc", test_to_array_non_malloc);

   g_test_add_func ("/bytes/null", test_null);

-- 

2.31.1