diff --git a/glib/gvariant-parser.c b/glib/gvariant-parser.c index c3090714f..7973ecf61 100644 --- a/glib/gvariant-parser.c +++ b/glib/gvariant-parser.c @@ -31,6 +31,7 @@ #include "gstrfuncs.h" #include "gtestutils.h" #include "gvariant.h" +#include "glib/gvariant-core.h" #include "gvariant-internal.h" #include "gvarianttype.h" #include "gslice.h" @@ -869,23 +870,86 @@ maybe_wrapper (AST *ast, const GVariantType *type, GError **error) { - const GVariantType *t; - GVariant *value; - int depth; + const GVariantType *base_type; + GVariant *base_value; + GVariant *value = NULL; + unsigned int depth; + gboolean trusted; + GVariantTypeInfo *base_type_info = NULL; + gsize base_serialised_fixed_size, base_serialised_size, serialised_size, n_suffix_zeros; + guint8 *serialised = NULL; + GBytes *bytes = NULL; + gsize i; - for (depth = 0, t = type; - g_variant_type_is_maybe (t); - depth++, t = g_variant_type_element (t)); + for (depth = 0, base_type = type; + g_variant_type_is_maybe (base_type); + depth++, base_type = g_variant_type_element (base_type)); - value = ast->class->get_base_value (ast, t, error); + base_value = ast->class->get_base_value (ast, base_type, error); - if (value == NULL) - return NULL; + if (base_value == NULL || depth == 0) + return g_steal_pointer (&base_value); - while (depth--) - value = g_variant_new_maybe (NULL, value); + /* This is the equivalent of calling g_variant_new_maybe() in a loop enough + * times to match the number of nested maybe types in @type. It does the same + * in a single `GVariant` allocation, though. + * + * This avoids maybe_wrapper() becoming an attack vector where a malicious + * text-form variant can create a long array, and insert a typedecl for a + * deeply nested maybe type on one of its elements. This is achievable with a + * relatively short text form, but results in O(array length × typedecl depth) + * allocations. This is a denial of service attack. + * + * Instead of constructing a tree of `GVariant`s in tree-form to match the + * @ast, construct a single `GVariant` containing the serialised form of the + * maybe-wrappers and the base value that they contain. This is relatively + * straightforward: serialise the base value, and then append the correct + * number of zero bytes for the maybe-wrappers. + * + * This is a bit of a layering violation, unfortunately. + * + * By doing this, the typedecl depth variable is reduced to O(1). + */ + trusted = g_variant_is_trusted (base_value); - return value; + /* See https://developer.gnome.org/documentation/specifications/gvariant-specification-1.0.html#maybes + * + * The serialised form of a `Just x` is the serialised form of `x` if `x` is + * fixed-size, and the serialised form of `x` plus a trailing zero byte if `x` + * is variable-size. A `Maybe` variant is always variable-size, even if its + * child element is fixed-size, because it might be `Nothing`. This means that + * all the maybe-wrappers which are not the innermost are always serialised + * with one trailing zero byte each. + * + * The serialised form of a `Nothing` is an empty byte sequence, but that’s + * already handled above in the `base_value == NULL` case. + */ + base_type_info = g_variant_type_info_get (base_type); + g_variant_type_info_query (base_type_info, NULL, &base_serialised_fixed_size); + g_variant_type_info_unref (base_type_info); + + base_serialised_size = g_variant_get_size (base_value); + n_suffix_zeros = (base_serialised_fixed_size > 0) ? depth - 1 : depth; + g_assert (base_serialised_size <= G_MAXSIZE - n_suffix_zeros); + serialised_size = base_serialised_size + n_suffix_zeros; + + g_assert (serialised_size >= base_serialised_size); + + /* Serialise the base value. */ + serialised = g_malloc (serialised_size); + g_variant_store (base_value, serialised); + + /* Zero-out the suffix zeros to complete the serialisation of the maybe wrappers. */ + for (i = base_serialised_size; i < serialised_size; i++) + serialised[i] = 0; + + bytes = g_bytes_new_take (g_steal_pointer (&serialised), serialised_size); + value = g_variant_new_from_bytes (type, bytes, trusted); + g_bytes_unref (bytes); + + g_variant_unref (base_value); + + return g_steal_pointer (&value); } typedef struct diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index fcb01ff3d..573e788a6 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4219,6 +4219,82 @@ test_parser_recursion_typedecls (void) g_free (silly_array); } +static void +test_parser_recursion_maybes (void) +{ + const gchar *hello = "hello"; + struct + { + const gchar *text_form; /* (not nullable) */ + GVariant *expected_variant; /* (not nullable) (owned) */ + } + vectors[] = + { + { + /* fixed size base value */ + "@mmmu 5", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_uint32 (5))))) + }, + { + /* variable size base value */ + "@mmmas ['hello']", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_strv (&hello, 1))))) + }, + { + /* fixed size base value, unset */ + "@mmmu just just nothing", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_maybe (G_VARIANT_TYPE_UINT32, NULL)))) + }, + { + /* variable size base value, unset */ + "@mmmas just just nothing", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (NULL, g_variant_new_maybe (G_VARIANT_TYPE_STRING_ARRAY, NULL)))) + }, + { + /* fixed size base value, unset */ + "@mmmu just nothing", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (G_VARIANT_TYPE ("mu"), NULL))) + }, + { + /* variable size base value, unset */ + "@mmmas just nothing", + g_variant_ref_sink (g_variant_new_maybe (NULL, g_variant_new_maybe (G_VARIANT_TYPE ("mas"), NULL))) + }, + { + /* fixed size base value, unset */ + "@mmmu nothing", + g_variant_ref_sink (g_variant_new_maybe (G_VARIANT_TYPE ("mmu"), NULL)) + }, + { + /* variable size base value, unset */ + "@mmmas nothing", + g_variant_ref_sink (g_variant_new_maybe (G_VARIANT_TYPE ("mmas"), NULL)) + }, + }; + gsize i; + + g_test_summary ("Test that nested maybes are handled correctly when parsing text-form variants"); + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2782"); + + for (i = 0; i < G_N_ELEMENTS (vectors); i++) + { + GVariant *value = NULL; + GError *local_error = NULL; + + g_test_message ("Text form %" G_GSIZE_FORMAT ": %s", i, vectors[i].text_form); + + value = g_variant_parse (NULL, vectors[i].text_form, NULL, NULL, &local_error); + g_assert_no_error (local_error); + g_assert_nonnull (value); + + g_assert_cmpvariant (value, vectors[i].expected_variant); + + g_variant_unref (value); + + g_clear_pointer (&vectors[i].expected_variant, g_variant_unref); + } +} + static void test_parse_bad_format_char (void) { @@ -5205,6 +5281,7 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/parser/integer-bounds", test_parser_integer_bounds); g_test_add_func ("/gvariant/parser/recursion", test_parser_recursion); g_test_add_func ("/gvariant/parser/recursion/typedecls", test_parser_recursion_typedecls); + g_test_add_func ("/gvariant/parser/recursion/maybes", test_parser_recursion_maybes); g_test_add_func ("/gvariant/parse-failures", test_parse_failures); g_test_add_func ("/gvariant/parse-positional", test_parse_positional); g_test_add_func ("/gvariant/parse/subprocess/bad-format-char", test_parse_bad_format_char);