diff options
author | Krzesimir Nowak <qdlacz@gmail.com> | 2021-02-10 23:51:07 +0100 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2021-03-19 10:02:49 +0000 |
commit | 1ad8c5bf82e14d85ad1ff012c16c2abcd554f073 (patch) | |
tree | 58baabc9534ef790ef65f68a44f6660843247358 | |
parent | 24b944692b55bf8a894a753143f9a510fba7df03 (diff) | |
download | glib-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.c | 6 | ||||
-rw-r--r-- | glib/gbytes.c | 4 | ||||
-rw-r--r-- | glib/tests/bytes.c | 37 |
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); |