luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-01-13 15:56:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

gvariant: Don’t allow child elements of a tuple to overlap each other

Browse Source 
This is similar to the earlier commit which prevents child elements of a
variable-sized array from overlapping each other, but this time for
tuples. It is based heavily on ideas by William Manley.

Tuples are slightly different from variable-sized arrays in that they
contain a mixture of fixed and variable sized elements. All but one of
the variable sized elements have an entry in the frame offsets table.
This means that if we were to just check the ordering of the frame
offsets table, the variable sized elements could still overlap
interleaving fixed sized elements, which would be bad.

Therefore we have to check the elements rather than the frame offsets.

The logic of checking the elements up to the index currently being
requested, and caching the result in `ordered_offsets_up_to`, means that
the algorithmic cost implications are the same for this commit as for
variable-sized arrays: an O(N) cost for these checks is amortised out
over N accesses to O(1) per access.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2121
This commit is contained in:
Philip Withnall 2022-01-07 16:42:14 +00:00
parent 73d0aa81c2
commit 7cf6f5b691
5 changed files with 232 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
glib/gvariant-core.c
View File

@ -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

40
glib/gvariant-serialiser.c
View File

@ -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)

7
glib/gvariant-serialiser.h
View File

@ -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;

1
glib/gvariant.c
View File

@ -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);

181
glib/tests/gvariant.c
View File

@ -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);