diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index f25d47279..441937113 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -1,6 +1,7 @@ /* * Copyright © 2007, 2008 Ryan Lortie * Copyright © 2010 Codethink Limited + * Copyright © 2022 Endless OS Foundation, LLC * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -181,7 +182,7 @@ struct _GVariant * offsets themselves. * * This field is only relevant for arrays of non - * fixed width types. + * fixed width types and for tuples. * * .tree: Only valid when the instance is in tree form. * @@ -1141,6 +1142,9 @@ g_variant_get_child_value (GVariant *value, */ s_child = g_variant_serialised_get_child (serialised, index_); + /* Update the cached ordered_offsets_up_to, since @serialised will be thrown away when this function exits */ + value->contents.serialised.ordered_offsets_up_to = MAX (value->contents.serialised.ordered_offsets_up_to, serialised.ordered_offsets_up_to); + /* Check whether this would cause nesting too deep. If so, return a fake * child. The only situation we expect this to happen in is with a variant, * as all other deeply-nested types have a static type, and hence should diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index bc131b751..263b74048 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -944,6 +944,10 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) * for the tuple. See the notes in gvarianttypeinfo.h. */ +/* Note: This doesn’t guarantee that @out_member_end >= @out_member_start; that + * condition may not hold true for invalid serialised variants. The caller is + * responsible for checking the returned values and handling invalid ones + * appropriately. */ static void gvs_tuple_get_member_bounds (GVariantSerialised value, gsize index_, @@ -1030,6 +1034,42 @@ gvs_tuple_get_child (GVariantSerialised value, return child; } + /* If the requested @index_ is beyond the set of indices whose framing offsets + * have been checked, check the remaining offsets to see whether they’re + * normal (in order, no overlapping tuple elements). + * + * Unlike the checks in gvs_variable_sized_array_get_child(), we have to check + * all the tuple *elements* here, not just all the framing offsets, since + * tuples contain a mix of elements which use framing offsets and ones which + * don’t. None of them are allowed to overlap. */ + if (index_ > value.ordered_offsets_up_to) + { + gsize i, prev_i_end = 0; + + if (value.ordered_offsets_up_to > 0) + gvs_tuple_get_member_bounds (value, value.ordered_offsets_up_to - 1, offset_size, NULL, &prev_i_end); + + for (i = value.ordered_offsets_up_to; i <= index_; i++) + { + gsize i_start, i_end; + + gvs_tuple_get_member_bounds (value, i, offset_size, &i_start, &i_end); + + if (i_start > i_end || i_start < prev_i_end || i_end > value.size) + break; + + prev_i_end = i_end; + } + + value.ordered_offsets_up_to = i - 1; + } + + if (index_ > value.ordered_offsets_up_to) + { + /* Offsets are invalid somewhere, so return an empty child. */ + return child; + } + if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET) { if (offset_size * (member_info->i + 2) > value.size) diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h index a1bccb834..661bd8fd1 100644 --- a/glib/gvariant-serialiser.h +++ b/glib/gvariant-serialiser.h @@ -37,8 +37,11 @@ typedef struct * This guarantees that the bytes of element n don't overlap with any previous * element. * - * This is both read and set by g_variant_serialised_get_child for arrays of - * non-fixed-width types */ + * This is both read and set by g_variant_serialised_get_child() for arrays of + * non-fixed-width types, and for tuples. + * + * Even when dealing with tuples, @ordered_offsets_up_to is an element index, + * rather than an index into the frame offsets. */ gsize ordered_offsets_up_to; } GVariantSerialised; diff --git a/glib/gvariant.c b/glib/gvariant.c index e4c85ca63..4f0d6b83c 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -6011,6 +6011,7 @@ g_variant_byteswap (GVariant *value) serialised.size = g_variant_get_size (trusted); serialised.data = g_malloc (serialised.size); serialised.depth = g_variant_get_depth (trusted); + serialised.ordered_offsets_up_to = G_MAXSIZE; /* operating on the normal form */ g_variant_store (trusted, serialised.data); g_variant_unref (trusted); diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 45e302c92..4ae52e2bb 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -1,6 +1,7 @@ /* * Copyright © 2010 Codethink Limited * Copyright © 2020 William Manley + * Copyright © 2022 Endless OS Foundation, LLC * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -1449,6 +1450,7 @@ test_maybe (void) serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; serialised.depth = 0; + serialised.ordered_offsets_up_to = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, @@ -1572,6 +1574,7 @@ test_array (void) serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; serialised.depth = 0; + serialised.ordered_offsets_up_to = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) instances, n_children); @@ -1736,6 +1739,7 @@ test_tuple (void) serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; serialised.depth = 0; + serialised.ordered_offsets_up_to = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) instances, n_children); @@ -1832,6 +1836,7 @@ test_variant (void) serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; serialised.depth = 0; + serialised.ordered_offsets_up_to = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) &instance, 1); @@ -5210,6 +5215,176 @@ test_normal_checking_tuple_offsets (void) g_variant_unref (variant); } +/* This is a regression test that we can't have non-normal values that take up + * significantly more space than the normal equivalent, by specifying the + * offset table entries so that tuple elements overlap. + * + * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838503 and + * https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838513 */ +static void +test_normal_checking_tuple_offsets2 (void) +{ + const GVariantType *data_type = G_VARIANT_TYPE ("(yyaiyyaiyy)"); + const guint8 data[] = { + 0x12, 0x34, 0x56, 0x78, 0x01, + /* + ^───────────────────┘ + + ^^^^^^^^^^ 1st yy + ^^^^^^^^^^ 2nd yy + ^^^^^^^^^^ 3rd yy + ^^^^ Framing offsets + */ + + /* If this variant was encoded normally, it would be something like this: + * 0x12, 0x34, pad, pad, [array bytes], 0x56, 0x78, pad, pad, [array bytes], 0x9A, 0xBC, 0xXX + * ^─────────────────────────────────────────────────────┘ + * + * ^^^^^^^^^^ 1st yy + * ^^^^^^^^^^ 2nd yy + * ^^^^^^^^^^ 3rd yy + * ^^^^ Framing offsets + */ + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + GVariant *expected = NULL; + + variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); + g_assert_nonnull (variant); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); + + expected = g_variant_new_parsed ( + "@(yyaiyyaiyy) (0x12, 0x34, [], 0x00, 0x00, [], 0x00, 0x00)"); + g_assert_cmpvariant (expected, variant); + g_assert_cmpvariant (expected, normal_variant); + + g_variant_unref (expected); + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + +/* This is a regression test that overlapping entries in the offset table are + * decoded consistently, even though they’re non-normal. + * + * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */ +static void +test_normal_checking_tuple_offsets3 (void) +{ + /* The expected decoding of this non-normal byte stream is complex. See + * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant + * specification. + * + * The rule “Child Values Overlapping Framing Offsets” from the specification + * says that the first `ay` must be decoded as `[0x01]` even though it + * overlaps the first byte of the offset table. However, since commit + * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow + * this as it’s exploitable. So the first `ay` must be given a default value. + * + * The second and third `ay`s must be given default values because of rule + * “End Boundary Precedes Start Boundary”. + * + * The `i` must be given a default value because of rule “Start or End + * Boundary of a Child Falls Outside the Container”. + */ + const GVariantType *data_type = G_VARIANT_TYPE ("(ayayiay)"); + const guint8 data[] = { + 0x01, 0x00, 0x02, + /* + ^──┘ + + ^^^^^^^^^^ 1st ay, bytes 0-2 (but given a default value anyway, see above) + 2nd ay, bytes 2-0 + i, bytes 0-4 + 3rd ay, bytes 4-1 + ^^^^^^^^^^ Framing offsets + */ + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + GVariant *expected = NULL; + + variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); + g_assert_nonnull (variant); + + g_assert_false (g_variant_is_normal_form (variant)); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); + + expected = g_variant_new_parsed ("@(ayayiay) ([], [], 0, [])"); + g_assert_cmpvariant (expected, variant); + g_assert_cmpvariant (expected, normal_variant); + + g_variant_unref (expected); + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + +/* This is a regression test that overlapping entries in the offset table are + * decoded consistently, even though they’re non-normal. + * + * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */ +static void +test_normal_checking_tuple_offsets4 (void) +{ + /* The expected decoding of this non-normal byte stream is complex. See + * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant + * specification. + * + * The rule “Child Values Overlapping Framing Offsets” from the specification + * says that the first `ay` must be decoded as `[0x01]` even though it + * overlaps the first byte of the offset table. However, since commit + * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow + * this as it’s exploitable. So the first `ay` must be given a default value. + * + * The second `ay` must be given a default value because of rule “End Boundary + * Precedes Start Boundary”. + * + * The third `ay` must be given a default value because its framing offsets + * overlap that of the first `ay`. + */ + const GVariantType *data_type = G_VARIANT_TYPE ("(ayayay)"); + const guint8 data[] = { + 0x01, 0x00, 0x02, + /* + ^──┘ + + ^^^^^^^^^^ 1st ay, bytes 0-2 (but given a default value anyway, see above) + 2nd ay, bytes 2-0 + 3rd ay, bytes 0-1 + ^^^^^^^^^^ Framing offsets + */ + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + GVariant *expected = NULL; + + variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); + g_assert_nonnull (variant); + + g_assert_false (g_variant_is_normal_form (variant)); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); + + expected = g_variant_new_parsed ("@(ayayay) ([], [], [])"); + g_assert_cmpvariant (expected, variant); + g_assert_cmpvariant (expected, normal_variant); + + g_variant_unref (expected); + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + /* Test that an empty object path is normalised successfully to the base object * path, ‘/’. */ static void @@ -5358,6 +5533,12 @@ main (int argc, char **argv) test_normal_checking_array_offsets2); g_test_add_func ("/gvariant/normal-checking/tuple-offsets", test_normal_checking_tuple_offsets); + g_test_add_func ("/gvariant/normal-checking/tuple-offsets2", + test_normal_checking_tuple_offsets2); + g_test_add_func ("/gvariant/normal-checking/tuple-offsets3", + test_normal_checking_tuple_offsets3); + g_test_add_func ("/gvariant/normal-checking/tuple-offsets4", + test_normal_checking_tuple_offsets4); g_test_add_func ("/gvariant/normal-checking/empty-object-path", test_normal_checking_empty_object_path);