diff options
author | Philip Withnall <philip@tecnocode.co.uk> | 2022-12-21 21:25:20 +0000 |
---|---|---|
committer | Philip Withnall <philip@tecnocode.co.uk> | 2022-12-21 21:25:20 +0000 |
commit | ac459f156d540fac0c35dca5c6a4fd7a6966c063 (patch) | |
tree | 3341a8f99d09c524a7c64f29e411be98e57fa645 | |
parent | 5e378c775a79b61f307f2b780652745b528b4105 (diff) | |
parent | 21a204147b16539b3eda3143b32844c49e29f4d4 (diff) | |
download | glib-ac459f156d540fac0c35dca5c6a4fd7a6966c063.tar.gz |
Merge branch '2840-2841-variant-more-fixes' into 'main'
gvariant: Check offset table doesn’t fall outside variant bounds and speed up text parsing
Closes #2840 and #2841
See merge request GNOME/glib!3163
-rw-r--r-- | glib/gvariant-core.c | 4 | ||||
-rw-r--r-- | glib/gvariant-serialiser.c | 12 | ||||
-rw-r--r-- | glib/tests/gvariant.c | 63 |
3 files changed, 74 insertions, 5 deletions
diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index f441c4757..477802282 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -1198,8 +1198,8 @@ g_variant_get_child_value (GVariant *value, child->contents.serialised.bytes = g_bytes_ref (value->contents.serialised.bytes); child->contents.serialised.data = s_child.data; - child->contents.serialised.ordered_offsets_up_to = s_child.ordered_offsets_up_to; - child->contents.serialised.checked_offsets_up_to = s_child.checked_offsets_up_to; + child->contents.serialised.ordered_offsets_up_to = (value->state & STATE_TRUSTED) ? G_MAXSIZE : s_child.ordered_offsets_up_to; + child->contents.serialised.checked_offsets_up_to = (value->state & STATE_TRUSTED) ? G_MAXSIZE : s_child.checked_offsets_up_to; return child; } diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index f443c2eb8..4e4a73ad1 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -984,7 +984,8 @@ gvs_tuple_get_member_bounds (GVariantSerialised value, member_info = g_variant_type_info_member_info (value.type_info, index_); - if (member_info->i + 1) + if (member_info->i + 1 && + offset_size * (member_info->i + 1) <= value.size) member_start = gvs_read_unaligned_le (value.data + value.size - offset_size * (member_info->i + 1), offset_size); @@ -995,7 +996,8 @@ gvs_tuple_get_member_bounds (GVariantSerialised value, member_start &= member_info->b; member_start |= member_info->c; - if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST) + if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST && + offset_size * (member_info->i + 1) <= value.size) member_end = value.size - offset_size * (member_info->i + 1); else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_FIXED) @@ -1006,11 +1008,15 @@ gvs_tuple_get_member_bounds (GVariantSerialised value, member_end = member_start + fixed_size; } - else /* G_VARIANT_MEMBER_ENDING_OFFSET */ + else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET && + offset_size * (member_info->i + 2) <= value.size) member_end = gvs_read_unaligned_le (value.data + value.size - offset_size * (member_info->i + 2), offset_size); + else /* invalid */ + member_end = G_MAXSIZE; + if (out_member_start != NULL) *out_member_start = member_start; if (out_member_end != NULL) diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index b360888e4..98c51a1d7 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -5576,6 +5576,67 @@ test_normal_checking_tuple_offsets4 (void) g_variant_unref (variant); } +/* This is a regression test that dereferencing the first element in the offset + * table doesn’t dereference memory before the start of the GVariant. The first + * element in the offset table gives the offset of the final member in the + * tuple (the offset table is stored in reverse), and the position of this final + * member is needed to check that none of the tuple members overlap with the + * offset table + * + * See https://gitlab.gnome.org/GNOME/glib/-/issues/2840 */ +static void +test_normal_checking_tuple_offsets5 (void) +{ + /* A tuple of type (sss) in normal form would have an offset table with two + * entries: + * - The first entry (lowest index in the table) gives the offset of the + * third `s` in the tuple, as the offset table is reversed compared to the + * tuple members. + * - The second entry (highest index in the table) gives the offset of the + * second `s` in the tuple. + * - The offset of the first `s` in the tuple is always 0. + * + * See §2.5.4 (Structures) of the GVariant specification for details, noting + * that the table is only layed out this way because all three members of the + * tuple have non-fixed sizes. + * + * It’s not clear whether the 0xaa data of this variant is part of the strings + * in the tuple, or part of the offset table. It doesn’t really matter. This + * is a regression test to check that the code to validate the offset table + * doesn’t unconditionally try to access the first entry in the offset table + * by subtracting the table size from the end of the GVariant data. + * + * In this non-normal case, that would result in an address off the start of + * the GVariant data, and an out-of-bounds read, because the GVariant is one + * byte long, but the offset table is calculated as two bytes long (with 1B + * sized entries) from the tuple’s type. + */ + const GVariantType *data_type = G_VARIANT_TYPE ("(sss)"); + const guint8 data[] = { 0xaa }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + GVariant *expected = NULL; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2840"); + + 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); + + expected = g_variant_new_parsed ("('', '', '')"); + 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 otherwise-valid serialised GVariant is considered non-normal if * its offset table entries are too wide. * @@ -5827,6 +5888,8 @@ main (int argc, char **argv) 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/tuple-offsets5", + test_normal_checking_tuple_offsets5); g_test_add_func ("/gvariant/normal-checking/tuple-offsets/minimal-sized", test_normal_checking_tuple_offsets_minimal_sized); g_test_add_func ("/gvariant/normal-checking/empty-object-path", |