summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKrzesimir Nowak <qdlacz@gmail.com>2021-02-10 23:51:07 +0100
committerSimon McVittie <smcv@collabora.com>2021-03-19 10:02:49 +0000
commit1ad8c5bf82e14d85ad1ff012c16c2abcd554f073 (patch)
tree58baabc9534ef790ef65f68a44f6660843247358
parent24b944692b55bf8a894a753143f9a510fba7df03 (diff)
downloadglib-wip/2-58-cve-2021-27218.tar.gz
gbytearray: Do not accept too large byte arrayswip/2-58-cve-2021-27218
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.) (Backport 2.66: Add #include gstrfuncsprivate.h in the test case for `g_memdup2()`.) (cherry picked from commit 0f384c88a241bbbd884487b1c40b7b75f1e638d3) Fixes: CVE-2021-27218
-rw-r--r--glib/garray.c6
-rw-r--r--glib/gbytes.c4
-rw-r--r--glib/tests/bytes.c37
3 files changed, 45 insertions, 2 deletions
diff --git a/glib/garray.c b/glib/garray.c
index a6cbd57bb..b00033a57 100644
--- a/glib/garray.c
+++ b/glib/garray.c
@@ -1755,6 +1755,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
@@ -1766,6 +1770,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 7b72886e5..d56abe6c3 100644
--- a/glib/gbytes.c
+++ b/glib/gbytes.c
@@ -519,6 +519,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..9e2638291 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
@@ -334,6 +334,38 @@ test_to_array_transferred (void)
}
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)
{
gconstpointer memory;
@@ -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);