Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP5:Update
glib2.11800
glib2-bsc1111499-gvariant-gdbus-fixes.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File glib2-bsc1111499-gvariant-gdbus-fixes.patch of Package glib2.11800
From 0f2087f243cf53e480f40880196c750026dfbca8 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 16 Aug 2018 20:12:02 +0100 Subject: [PATCH 01/15] gvariant: Fix checking arithmetic for tuple element ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When checking whether a serialised GVariant tuple is in normal form, it’s possible for `offset_ptr -= offset_size` to underflow and wrap around, resulting in gvs_read_unaligned_le() reading memory outside the serialised GVariant bounds. See §(Tuples) in gvariant-serialiser.c for the documentation on how tuples are serialised. Briefly, all variable-length elements in the tuple have an offset to their end stored in an array of offsets at the end of the tuple. The width of each offset is in offset_size. offset_ptr is added to the start of the serialised tuple to get the offset which is currently being examined. The offset array is in reverse order compared to the tuple elements, hence the subtraction. The bug can be triggered if a tuple contains a load of variable-length elements, each of whose length is actually zero (i.e. empty arrays). Includes a unit test. oss-fuzz#9801 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- glib/gvariant-serialiser.c | 3 +++ glib/tests/gvariant.c | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index 5df7fc3..bd582e7 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -1065,6 +1065,9 @@ gvs_tuple_is_normal (GVariantSerialised value) break; case G_VARIANT_MEMBER_ENDING_OFFSET: + if (offset_ptr < offset_size) + return FALSE; + offset_ptr -= offset_size; if (offset_ptr < offset) diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 889b28b..66f0b88 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4533,6 +4533,30 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS G_GNUC_END_IGNORE_DEPRECATIONS } +/* Test checking arbitrary binary data for normal form. This time, it’s a tuple + * with invalid element ends. */ +static void +test_normal_checking_tuples (void) +{ + const guint8 data[] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, + 'a', '(', 'a', 'o', 'a', 'o', 'a', 'a', 'o', 'a', 'a', 'o', ')' + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + + variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, data, size, + FALSE, NULL, NULL); + g_assert_nonnull (variant); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + int main (int argc, char **argv) { @@ -4592,5 +4616,8 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/print-context", test_print_context); g_test_add_func ("/gvariant/error-quark", test_error_quark); + g_test_add_func ("/gvariant/normal-checking/tuples", + test_normal_checking_tuples); + return g_test_run (); } -- 2.16.3 From 82109efdfbbaaa24ad4c2ae338a135368cd4079e Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Fri, 7 Sep 2018 20:27:39 +0100 Subject: [PATCH 02/15] gvarianttype: Impose a recursion limit of 64 on variant types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, GVariant has allowed ‘arbitrary’ recursion on GVariantTypes, but this isn’t really feasible. We have to deal with GVariants from untrusted sources, and the nature of GVariantType means that another level of recursion (and hence, for example, another stack frame in your application) can be added with a single byte in a variant type signature in the input. This gives malicious input sources far too much leverage to cause deep stack recursion or massive memory allocations which can DoS an application. Limit recursion to 64 levels (which should be more than enough for anyone™), document it and add a test. This is, handily, also the limit applied by the D-Bus specification (§(Valid Signatures)). oss-fuzz#9857 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- docs/reference/glib/glib-sections.txt | 1 + glib/gvariant-core.c | 46 +++++++++++- glib/gvariant-internal.h | 18 +++++ glib/gvariant-serialiser.c | 38 ++++++++-- glib/gvariant-serialiser.h | 1 + glib/gvarianttype.c | 137 +++++++++++++++++++++++++--------- glib/gvarianttype.h | 2 + glib/gvarianttypeinfo.c | 25 +++++++ glib/gvarianttypeinfo.h | 2 + glib/tests/gvariant.c | 69 +++++++++++++++++ 10 files changed, 295 insertions(+), 44 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 4dbdd06..23abefa 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3362,6 +3362,7 @@ g_variant_parse_error_print_context g_variant_parse_error_quark g_variant_parser_get_error_quark g_variant_type_checked_ +g_variant_type_string_get_depth_ </SECTION> diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index 8301250..5df1350 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -20,6 +20,7 @@ #include <glib/gvariant-core.h> +#include <glib/gvariant-internal.h> #include <glib/gvariant-serialiser.h> #include <glib/gtestutils.h> #include <glib/gbitlock.h> @@ -74,6 +75,7 @@ struct _GVariant gint state; gint ref_count; + gsize depth; }; /* struct GVariant: @@ -202,6 +204,11 @@ struct _GVariant * reference. See g_variant_ref_sink(). * * ref_count: the reference count of the instance + * + * depth: the depth of the GVariant in a hierarchy of nested containers, + * increasing with the level of nesting. The top-most GVariant has depth + * zero. This is used to avoid recursing too deeply and overflowing the + * stack when handling deeply nested untrusted serialised GVariants. */ #define STATE_LOCKED 1 #define STATE_SERIALISED 2 @@ -366,6 +373,7 @@ g_variant_serialise (GVariant *value, serialised.type_info = value->type_info; serialised.size = value->size; serialised.data = data; + serialised.depth = value->depth; children = (gpointer *) value->contents.tree.children; n_children = value->contents.tree.n_children; @@ -407,6 +415,7 @@ g_variant_fill_gvs (GVariantSerialised *serialised, if (serialised->size == 0) serialised->size = value->size; g_assert (serialised->size == value->size); + serialised->depth = value->depth; if (serialised->data) /* g_variant_store() is a public API, so it @@ -480,6 +489,7 @@ g_variant_alloc (const GVariantType *type, STATE_FLOATING; value->size = (gssize) -1; value->ref_count = 1; + value->depth = 0; return value; } @@ -933,7 +943,8 @@ g_variant_n_children (GVariant *value) GVariantSerialised serialised = { value->type_info, (gpointer) value->contents.serialised.data, - value->size + value->size, + value->depth, }; n_children = g_variant_serialised_n_children (serialised); @@ -962,6 +973,11 @@ g_variant_n_children (GVariant *value) * The returned value is never floating. You should free it with * g_variant_unref() when you're done with it. * + * There may be implementation specific restrictions on deeply nested values, + * which would result in the unit tuple being returned as the child value, + * instead of further nested children. #GVariant is guaranteed to handle + * nesting up to at least 64 levels. + * * This function is O(1). * * Returns: (transfer full): the child at the specified index @@ -973,6 +989,7 @@ g_variant_get_child_value (GVariant *value, gsize index_) { g_return_val_if_fail (index_ < g_variant_n_children (value), NULL); + g_return_val_if_fail (value->depth < G_MAXSIZE, NULL); if (~g_atomic_int_get (&value->state) & STATE_SERIALISED) { @@ -995,7 +1012,8 @@ g_variant_get_child_value (GVariant *value, GVariantSerialised serialised = { value->type_info, (gpointer) value->contents.serialised.data, - value->size + value->size, + value->depth, }; GVariantSerialised s_child; GVariant *child; @@ -1005,6 +1023,20 @@ g_variant_get_child_value (GVariant *value, */ s_child = g_variant_serialised_get_child (serialised, index_); + /* 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 + * have been rejected earlier. In the case of a variant whose nesting plus + * the depth of its child is too great, return a unit variant () instead of + * the real child. */ + if (!(value->state & STATE_TRUSTED) && + g_variant_type_info_query_depth (s_child.type_info) >= + G_VARIANT_MAX_RECURSION_DEPTH - value->depth) + { + g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_VARIANT)); + return g_variant_new_tuple (NULL, 0); + } + /* create a new serialised instance out of it */ child = g_slice_new (GVariant); child->type_info = s_child.type_info; @@ -1012,6 +1044,7 @@ g_variant_get_child_value (GVariant *value, STATE_SERIALISED; child->size = s_child.size; child->ref_count = 1; + child->depth = value->depth + 1; child->contents.serialised.bytes = g_bytes_ref (value->contents.serialised.bytes); child->contents.serialised.data = s_child.data; @@ -1074,6 +1107,9 @@ g_variant_store (GVariant *value, * being trusted. If the value was already marked as being trusted then * this function will immediately return %TRUE. * + * There may be implementation specific restrictions on deeply nested values. + * GVariant is guaranteed to handle nesting up to at least 64 levels. + * * Returns: %TRUE if @value is in normal form * * Since: 2.24 @@ -1086,12 +1122,16 @@ g_variant_is_normal_form (GVariant *value) g_variant_lock (value); + if (value->depth >= G_VARIANT_MAX_RECURSION_DEPTH) + return FALSE; + if (value->state & STATE_SERIALISED) { GVariantSerialised serialised = { value->type_info, (gpointer) value->contents.serialised.data, - value->size + value->size, + value->depth }; if (g_variant_serialised_is_normal (serialised)) diff --git a/glib/gvariant-internal.h b/glib/gvariant-internal.h index 6bf19ad..f6ee003 100644 --- a/glib/gvariant-internal.h +++ b/glib/gvariant-internal.h @@ -47,4 +47,22 @@ GVariantType * g_variant_format_string_scan_type (const g const gchar *limit, const gchar **endptr); +/* The maximum number of levels of nested container which this implementation + * of #GVariant will handle. + * + * The limit must be at least 64 + 1, to allow D-Bus messages to be wrapped in + * a top-level #GVariant. This comes from the D-Bus specification (§(Valid + * Signatures)), but also seems generally reasonable. #GDBusMessage wraps its + * payload in a top-level tuple. + * + * The limit is actually set to be a lot greater than 64, to allow much greater + * nesting of values off D-Bus. It cannot be set over around 80000, or the risk + * of overflowing the stack when parsing type strings becomes too great. + * + * Aside from those constraints, the choice of this value is arbitrary. The + * only restrictions on it from the API are that it has to be greater than 64 + * (due to D-Bus). +*/ +#define G_VARIANT_MAX_RECURSION_DEPTH ((gsize) 128) + #endif /* __G_VARIANT_INTERNAL_H__ */ diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index bd582e7..a6df4d3 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -23,6 +23,7 @@ #include "gvariant-serialiser.h" +#include <glib/gvariant-internal.h> #include <glib/gtestutils.h> #include <glib/gstrfuncs.h> #include <glib/gtypes.h> @@ -81,7 +82,9 @@ * values is permitted (eg: 0 to 255 is a valid byte). Special checks * need to be performed for booleans (only 0 or 1 allowed), strings * (properly nul-terminated) and object paths and signature strings - * (meeting the D-Bus specification requirements). + * (meeting the D-Bus specification requirements). Depth checks need to be + * performed for nested types (arrays, tuples, and variants), to avoid massive + * recursion which could exhaust our stack when handling untrusted input. */ /* < private > @@ -113,6 +116,9 @@ * fixed-sized type, yet @size must be non-zero. The effect of this * combination should be as if @data were a pointer to an * appropriately-sized zero-filled region. + * + * @depth has no restrictions; the depth of a top-level serialised #GVariant is + * zero, and it increases for each level of nested child. */ /* < private > @@ -255,6 +261,7 @@ gvs_fixed_sized_maybe_get_child (GVariantSerialised value, */ value.type_info = g_variant_type_info_element (value.type_info); g_variant_type_info_ref (value.type_info); + value.depth++; return value; } @@ -286,7 +293,7 @@ gvs_fixed_sized_maybe_serialise (GVariantSerialised value, { if (n_children) { - GVariantSerialised child = { NULL, value.data, value.size }; + GVariantSerialised child = { NULL, value.data, value.size, value.depth + 1 }; gvs_filler (&child, children[0]); } @@ -307,6 +314,7 @@ gvs_fixed_sized_maybe_is_normal (GVariantSerialised value) /* proper element size: "Just". recurse to the child. */ value.type_info = g_variant_type_info_element (value.type_info); + value.depth++; return g_variant_serialised_is_normal (value); } @@ -347,6 +355,8 @@ gvs_variable_sized_maybe_get_child (GVariantSerialised value, if (value.size == 0) value.data = NULL; + value.depth++; + return value; } @@ -376,7 +386,7 @@ gvs_variable_sized_maybe_serialise (GVariantSerialised value, { if (n_children) { - GVariantSerialised child = { NULL, value.data, value.size - 1 }; + GVariantSerialised child = { NULL, value.data, value.size - 1, value.depth + 1 }; /* write the data for the child. */ gvs_filler (&child, children[0]); @@ -395,6 +405,7 @@ gvs_variable_sized_maybe_is_normal (GVariantSerialised value) value.type_info = g_variant_type_info_element (value.type_info); value.size--; + value.depth++; return g_variant_serialised_is_normal (value); } @@ -445,6 +456,7 @@ gvs_fixed_sized_array_get_child (GVariantSerialised value, g_variant_type_info_query (child.type_info, NULL, &child.size); child.data = value.data + (child.size * index_); g_variant_type_info_ref (child.type_info); + child.depth = value.depth + 1; return child; } @@ -474,6 +486,7 @@ gvs_fixed_sized_array_serialise (GVariantSerialised value, child.type_info = g_variant_type_info_element (value.type_info); g_variant_type_info_query (child.type_info, NULL, &child.size); child.data = value.data; + child.depth = value.depth + 1; for (i = 0; i < n_children; i++) { @@ -489,6 +502,7 @@ gvs_fixed_sized_array_is_normal (GVariantSerialised value) child.type_info = g_variant_type_info_element (value.type_info); g_variant_type_info_query (child.type_info, NULL, &child.size); + child.depth = value.depth + 1; if (value.size % child.size != 0) return FALSE; @@ -655,6 +669,7 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, child.type_info = g_variant_type_info_element (value.type_info); g_variant_type_info_ref (child.type_info); + child.depth = value.depth + 1; offset_size = gvs_get_offset_size (value.size); @@ -783,6 +798,7 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) child.type_info = g_variant_type_info_element (value.type_info); g_variant_type_info_query (child.type_info, &alignment, NULL); + child.depth = value.depth + 1; offset = 0; for (i = 0; i < length; i++) @@ -858,6 +874,7 @@ gvs_tuple_get_child (GVariantSerialised value, member_info = g_variant_type_info_member_info (value.type_info, index_); child.type_info = g_variant_type_info_ref (member_info->type_info); + child.depth = value.depth + 1; offset_size = gvs_get_offset_size (value.size); /* tuples are the only (potentially) fixed-sized containers, so the @@ -1042,6 +1059,7 @@ gvs_tuple_is_normal (GVariantSerialised value) member_info = g_variant_type_info_member_info (value.type_info, i); child.type_info = member_info->type_info; + child.depth = value.depth + 1; g_variant_type_info_query (child.type_info, &alignment, &fixed_size); @@ -1171,8 +1189,10 @@ gvs_variant_get_child (GVariantSerialised value, if (g_variant_type_is_definite (type)) { gsize fixed_size; + gsize child_type_depth; child.type_info = g_variant_type_info_get (type); + child.depth = value.depth + 1; if (child.size != 0) /* only set to non-%NULL if size > 0 */ @@ -1180,8 +1200,10 @@ gvs_variant_get_child (GVariantSerialised value, g_variant_type_info_query (child.type_info, NULL, &fixed_size); + child_type_depth = g_variant_type_info_query_depth (child.type_info); - if (!fixed_size || fixed_size == child.size) + if ((!fixed_size || fixed_size == child.size) && + value.depth < G_VARIANT_MAX_RECURSION_DEPTH - child_type_depth) return child; g_variant_type_info_unref (child.type_info); @@ -1193,6 +1215,7 @@ gvs_variant_get_child (GVariantSerialised value, child.type_info = g_variant_type_info_get (G_VARIANT_TYPE_UNIT); child.data = NULL; child.size = 1; + child.depth = value.depth + 1; return child; } @@ -1234,10 +1257,13 @@ gvs_variant_is_normal (GVariantSerialised value) { GVariantSerialised child; gboolean normal; + gsize child_type_depth; child = gvs_variant_get_child (value, 0); + child_type_depth = g_variant_type_info_query_depth (child.type_info); - normal = (child.data != NULL || child.size == 0) && + normal = (value.depth < G_VARIANT_MAX_RECURSION_DEPTH - child_type_depth) && + (child.data != NULL || child.size == 0) && g_variant_serialised_is_normal (child); g_variant_type_info_unref (child.type_info); @@ -1555,6 +1581,8 @@ g_variant_serialised_is_normal (GVariantSerialised serialised) if (serialised.data == NULL) return FALSE; + if (serialised.depth >= G_VARIANT_MAX_RECURSION_DEPTH) + return FALSE; /* some hard-coded terminal cases */ switch (g_variant_type_info_get_type_char (serialised.type_info)) diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h index 2be3299..6360a42 100644 --- a/glib/gvariant-serialiser.h +++ b/glib/gvariant-serialiser.h @@ -28,6 +28,7 @@ typedef struct GVariantTypeInfo *type_info; guchar *data; gsize size; + gsize depth; /* same semantics as GVariant.depth */ } GVariantSerialised; /* deserialisation */ diff --git a/glib/gvarianttype.c b/glib/gvarianttype.c index b3e227a..359fdf2 100644 --- a/glib/gvarianttype.c +++ b/glib/gvarianttype.c @@ -24,6 +24,7 @@ #include <glib/gtestutils.h> #include <glib/gstrfuncs.h> +#include <glib/gvariant-internal.h> #include <string.h> @@ -114,7 +115,10 @@ * * The above definition is recursive to arbitrary depth. "aaaaai" and * "(ui(nq((y)))s)" are both valid type strings, as is - * "a(aa(ui)(qna{ya(yd)}))". + * "a(aa(ui)(qna{ya(yd)}))". In order to not hit memory limits, #GVariant + * imposes a limit on recursion depth of 65 nested containers. This is the + * limit in the D-Bus specification (64) plus one to allow a #GDBusMessage to + * be nested in a top-level tuple. * * The meaning of each of the characters is as follows: * - `b`: the type string of %G_VARIANT_TYPE_BOOLEAN; a boolean value. @@ -194,35 +198,15 @@ g_variant_type_check (const GVariantType *type) #endif } -/** - * g_variant_type_string_scan: - * @string: a pointer to any string - * @limit: (allow-none): the end of @string, or %NULL - * @endptr: (out) (allow-none): location to store the end pointer, or %NULL - * - * Scan for a single complete and valid GVariant type string in @string. - * The memory pointed to by @limit (or bytes beyond it) is never - * accessed. - * - * If a valid type string is found, @endptr is updated to point to the - * first character past the end of the string that was found and %TRUE - * is returned. - * - * If there is no valid type string starting at @string, or if the type - * string does not end before @limit then %FALSE is returned. - * - * For the simple case of checking if a string is a valid type string, - * see g_variant_type_string_is_valid(). - * - * Returns: %TRUE if a valid type string was found - * - * Since: 2.24 - **/ -gboolean -g_variant_type_string_scan (const gchar *string, - const gchar *limit, - const gchar **endptr) +static gboolean +variant_type_string_scan_internal (const gchar *string, + const gchar *limit, + const gchar **endptr, + gsize *depth, + gsize depth_limit) { + gsize max_depth = 0, child_depth; + g_return_val_if_fail (string != NULL, FALSE); if (string == limit || *string == '\0') @@ -232,27 +216,44 @@ g_variant_type_string_scan (const gchar *string, { case '(': while (string == limit || *string != ')') - if (!g_variant_type_string_scan (string, limit, &string)) - return FALSE; + { + if (depth_limit == 0 || + !variant_type_string_scan_internal (string, limit, &string, + &child_depth, + depth_limit - 1)) + return FALSE; + + max_depth = MAX (max_depth, child_depth + 1); + } string++; break; case '{': - if (string == limit || *string == '\0' || /* { */ - !strchr ("bynqihuxtdsog?", *string++) || /* key */ - !g_variant_type_string_scan (string, limit, &string) || /* value */ - string == limit || *string++ != '}') /* } */ + if (depth_limit == 0 || + string == limit || *string == '\0' || /* { */ + !strchr ("bynqihuxtdsog?", *string++) || /* key */ + !variant_type_string_scan_internal (string, limit, &string, + &child_depth, depth_limit - 1) || /* value */ + string == limit || *string++ != '}') /* } */ return FALSE; + max_depth = MAX (max_depth, child_depth + 1); break; case 'm': case 'a': - return g_variant_type_string_scan (string, limit, endptr); + if (depth_limit == 0 || + !variant_type_string_scan_internal (string, limit, &string, + &child_depth, depth_limit - 1)) + return FALSE; + + max_depth = MAX (max_depth, child_depth + 1); + break; case 'b': case 'y': case 'n': case 'q': case 'i': case 'u': case 'x': case 't': case 'd': case 's': case 'o': case 'g': case 'v': case 'r': case '*': case '?': case 'h': + max_depth = MAX (max_depth, 1); break; default: @@ -261,10 +262,74 @@ g_variant_type_string_scan (const gchar *string, if (endptr != NULL) *endptr = string; + if (depth != NULL) + *depth = max_depth; return TRUE; } +/** + * g_variant_type_string_scan: + * @string: a pointer to any string + * @limit: (nullable): the end of @string, or %NULL + * @endptr: (out) (optional): location to store the end pointer, or %NULL + * + * Scan for a single complete and valid GVariant type string in @string. + * The memory pointed to by @limit (or bytes beyond it) is never + * accessed. + * + * If a valid type string is found, @endptr is updated to point to the + * first character past the end of the string that was found and %TRUE + * is returned. + * + * If there is no valid type string starting at @string, or if the type + * string does not end before @limit then %FALSE is returned. + * + * For the simple case of checking if a string is a valid type string, + * see g_variant_type_string_is_valid(). + * + * Returns: %TRUE if a valid type string was found + * + * Since: 2.24 + **/ +gboolean +g_variant_type_string_scan (const gchar *string, + const gchar *limit, + const gchar **endptr) +{ + return variant_type_string_scan_internal (string, limit, endptr, NULL, + G_VARIANT_MAX_RECURSION_DEPTH); +} + +/* < private > + * g_variant_type_string_get_depth_: + * @type_string: a pointer to any string + * + * Get the maximum depth of the nested types in @type_string. A basic type will + * return depth 1, and a container type will return a greater value. The depth + * of a tuple is 1 plus the depth of its deepest child type. + * + * If @type_string is not a valid #GVariant type string, 0 will be returned. + * + * Returns: depth of @type_string, or 0 on error + * Since: 2.60 + */ +gsize +g_variant_type_string_get_depth_ (const gchar *type_string) +{ + const gchar *endptr; + gsize depth = 0; + + g_return_val_if_fail (type_string != NULL, 0); + + if (!variant_type_string_scan_internal (type_string, NULL, &endptr, &depth, + G_VARIANT_MAX_RECURSION_DEPTH) || + *endptr != '\0') + return 0; + + return depth; +} + /** * g_variant_type_string_is_valid: * @type_string: a pointer to any string diff --git a/glib/gvarianttype.h b/glib/gvarianttype.h index e3a562f..bd27c49 100644 --- a/glib/gvarianttype.h +++ b/glib/gvarianttype.h @@ -374,6 +374,8 @@ GVariantType * g_variant_type_new_dict_entry (const G /*< private >*/ GLIB_AVAILABLE_IN_ALL const GVariantType * g_variant_type_checked_ (const gchar *); +GLIB_AVAILABLE_IN_2_48 +gsize g_variant_type_string_get_depth_ (const gchar *type_string); G_END_DECLS diff --git a/glib/gvarianttypeinfo.c b/glib/gvarianttypeinfo.c index b398d65..7c6a0c2 100644 --- a/glib/gvarianttypeinfo.c +++ b/glib/gvarianttypeinfo.c @@ -251,6 +251,31 @@ g_variant_type_info_query (GVariantTypeInfo *info, *fixed_size = info->fixed_size; } +/* < private > + * g_variant_type_info_query_depth: + * @info: a #GVariantTypeInfo + * + * Queries @info to determine the depth of the type. + * + * See g_variant_type_string_get_depth_() for more details. + * + * Returns: depth of @info + * Since: 2.60 + */ +gsize +g_variant_type_info_query_depth (GVariantTypeInfo *info) +{ + g_variant_type_info_check (info, 0); + + if (info->container_class) + { + ContainerInfo *container = (ContainerInfo *) info; + return g_variant_type_string_get_depth_ (container->type_string); + } + + return 1; +} + /* == array == */ #define GV_ARRAY_INFO_CLASS 'a' static ArrayInfo * diff --git a/glib/gvarianttypeinfo.h b/glib/gvarianttypeinfo.h index 0714d78..32e9a1d 100644 --- a/glib/gvarianttypeinfo.h +++ b/glib/gvarianttypeinfo.h @@ -130,6 +130,8 @@ GLIB_AVAILABLE_IN_ALL void g_variant_type_info_query (GVariantTypeInfo *typeinfo, guint *alignment, gsize *size); +GLIB_AVAILABLE_IN_2_48 +gsize g_variant_type_info_query_depth (GVariantTypeInfo *typeinfo); /* array */ GLIB_AVAILABLE_IN_ALL diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 66f0b88..3cd0039 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -661,6 +661,57 @@ test_gvarianttype (void) } } +/* Test that scanning a deeply recursive type string doesn’t exhaust our + * stack space (which it would if the type string scanner was recursive). */ +static void +test_gvarianttype_string_scan_recursion_tuple (void) +{ + gchar *type_string = NULL; + gsize type_string_len = 1000001; /* not including nul terminator */ + gsize i; + + /* Build a long type string of ‘((…u…))’. */ + type_string = g_new0 (gchar, type_string_len + 1); + for (i = 0; i < type_string_len; i++) + { + if (i < type_string_len / 2) + type_string[i] = '('; + else if (i == type_string_len / 2) + type_string[i] = 'u'; + else + type_string[i] = ')'; + } + + /* Goes (way) over allowed recursion limit. */ + g_assert_false (g_variant_type_string_is_valid (type_string)); + + g_free (type_string); +} + +/* Same as above, except with an array rather than a tuple. */ +static void +test_gvarianttype_string_scan_recursion_array (void) +{ + gchar *type_string = NULL; + gsize type_string_len = 1000001; /* not including nul terminator */ + gsize i; + + /* Build a long type string of ‘aaa…aau’. */ + type_string = g_new0 (gchar, type_string_len + 1); + for (i = 0; i < type_string_len; i++) + { + if (i < type_string_len - 1) + type_string[i] = 'a'; + else + type_string[i] = 'u'; + } + + /* Goes (way) over allowed recursion limit. */ + g_assert_false (g_variant_type_string_is_valid (type_string)); + + g_free (type_string); +} + #define ALIGNED(x, y) (((x + (y - 1)) / y) * y) /* do our own calculation of the fixed_size and alignment of a type @@ -1230,6 +1281,8 @@ random_instance_filler (GVariantSerialised *serialised, if (serialised->size == 0) serialised->size = instance->size; + serialised->depth = 0; + g_assert (serialised->type_info == instance->type_info); g_assert (serialised->size == instance->size); @@ -1394,6 +1447,7 @@ test_maybe (void) serialised.type_info = type_info; serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; + serialised.depth = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, @@ -1516,6 +1570,7 @@ test_array (void) serialised.type_info = array_info; serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; + serialised.depth = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) instances, n_children); @@ -1679,6 +1734,7 @@ test_tuple (void) serialised.type_info = type_info; serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; + serialised.depth = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) instances, n_children); @@ -1774,6 +1830,7 @@ test_variant (void) serialised.type_info = type_info; serialised.data = flavoured_malloc (needed_size, flavour); serialised.size = needed_size; + serialised.depth = 0; g_variant_serialiser_serialise (serialised, random_instance_filler, (gpointer *) &instance, 1); @@ -2055,6 +2112,8 @@ tree_filler (GVariantSerialised *serialised, if (serialised->type_info == NULL) serialised->type_info = instance->info; + serialised->depth = 0; + if (instance->data_size == 0) /* is a container */ { @@ -2225,6 +2284,7 @@ test_byteswap (void) g_variant_serialised_byteswap (two); g_assert_cmpmem (one.data, one.size, two.data, two.size); + g_assert_cmpuint (one.depth, ==, two.depth); tree_instance_free (tree); g_free (one.data); @@ -4565,6 +4625,10 @@ main (int argc, char **argv) g_test_init (&argc, &argv, NULL); g_test_add_func ("/gvariant/type", test_gvarianttype); + g_test_add_func ("/gvariant/type/string-scan/recursion/tuple", + test_gvarianttype_string_scan_recursion_tuple); + g_test_add_func ("/gvariant/type/string-scan/recursion/array", + test_gvarianttype_string_scan_recursion_array); g_test_add_func ("/gvariant/typeinfo", test_gvarianttypeinfo); g_test_add_func ("/gvariant/serialiser/maybe", test_maybes); g_test_add_func ("/gvariant/serialiser/array", test_arrays); @@ -4619,5 +4683,10 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/normal-checking/tuples", test_normal_checking_tuples); + g_test_add_func ("/gvariant/recursion-limits/variant-in-variant", + test_recursion_limits_variant_in_variant); + g_test_add_func ("/gvariant/recursion-limits/array-in-variant", + test_recursion_limits_array_in_variant); + return g_test_run (); } -- 2.16.3 From 381aae00d23795423eb588bb772600304c293b1b Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Fri, 7 Sep 2018 22:26:05 +0100 Subject: [PATCH 03/15] gvariant: Check array offsets against serialised data length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When getting a child from a serialised variable array, check its offset against the length of the serialised data of the array (excluding the length of the offset table). The offset was already checked against the length of the entire serialised array (including the offset table) — but a child should not be able to start inside the offset table. A test is included. oss-fuzz#9803 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- glib/gvariant-serialiser.c | 2 +- glib/tests/gvariant.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index a6df4d3..3000c67 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -694,7 +694,7 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, (offset_size * index_), offset_size); - if (start < end && end <= value.size) + if (start < end && end <= value.size && end <= last_end) { child.data = value.data + start; child.size = end - start; diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 3cd0039..1ad3ba6 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4617,6 +4617,30 @@ test_normal_checking_tuples (void) g_variant_unref (variant); } +/* Test that an array with invalidly large values in its offset table is + * normalised successfully without looping infinitely. */ +static void +test_normal_checking_array_offsets (void) +{ + const guint8 data[] = { + 0x07, 0xe5, 0x00, 0x07, 0x00, 0x07, 0x00, 0x00, + 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'g', + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + + variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, data, size, + FALSE, NULL, NULL); + g_assert_nonnull (variant); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + int main (int argc, char **argv) { @@ -4682,6 +4706,8 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/normal-checking/tuples", test_normal_checking_tuples); + g_test_add_func ("/gvariant/normal-checking/array-offsets", + test_normal_checking_array_offsets); g_test_add_func ("/gvariant/recursion-limits/variant-in-variant", test_recursion_limits_variant_in_variant); -- 2.16.3 From 890b0b5b49ea7a9120fca3b0cd76f676e14885b5 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Fri, 7 Sep 2018 22:28:37 +0100 Subject: [PATCH 04/15] gvariant: Check tuple offsets against serialised data length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with the previous commit, when getting a child from a serialised tuple, check its offset against the length of the serialised data of the tuple (excluding the length of the offset table). The offset was already checked against the length of the entire serialised tuple (including the offset table) — but a child should not be able to start inside the offset table. A test is included. oss-fuzz#9803 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- glib/gvariant-serialiser.c | 16 ++++++++++++++-- glib/tests/gvariant.c | 26 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index 3000c67..48405c0 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -870,7 +870,7 @@ gvs_tuple_get_child (GVariantSerialised value, const GVariantMemberInfo *member_info; GVariantSerialised child = { 0, }; gsize offset_size; - gsize start, end; + gsize start, end, last_end; member_info = g_variant_type_info_member_info (value.type_info, index_); child.type_info = g_variant_type_info_ref (member_info->type_info); @@ -940,7 +940,19 @@ gvs_tuple_get_child (GVariantSerialised value, offset_size * (member_info->i + 2), offset_size); - if (start < end && end <= value.size) + /* The child should not extend into the offset table. */ + if (index_ != g_variant_type_info_n_members (value.type_info) - 1) + { + GVariantSerialised last_child; + last_child = gvs_tuple_get_child (value, + g_variant_type_info_n_members (value.type_info) - 1); + last_end = last_child.data + last_child.size - value.data; + g_variant_type_info_unref (last_child.type_info); + } + else + last_end = end; + + if (start < end && end <= value.size && end <= last_end) { child.data = value.data + start; child.size = end - start; diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 1ad3ba6..39c680f 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4641,6 +4641,30 @@ test_normal_checking_array_offsets (void) g_variant_unref (variant); } +/* Test that a tuple with invalidly large values in its offset table is + * normalised successfully without looping infinitely. */ +static void +test_normal_checking_tuple_offsets (void) +{ + const guint8 data[] = { + 0x07, 0xe5, 0x00, 0x07, 0x00, 0x07, + '(', 'a', 's', 'a', 's', 'a', 's', 'a', 's', 'a', 's', 'a', 's', ')', + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + + variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, data, size, + FALSE, NULL, NULL); + g_assert_nonnull (variant); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + int main (int argc, char **argv) { @@ -4708,6 +4732,8 @@ main (int argc, char **argv) test_normal_checking_tuples); g_test_add_func ("/gvariant/normal-checking/array-offsets", test_normal_checking_array_offsets); + g_test_add_func ("/gvariant/normal-checking/tuple-offsets", + test_normal_checking_tuple_offsets); g_test_add_func ("/gvariant/recursion-limits/variant-in-variant", test_recursion_limits_variant_in_variant); -- 2.16.3 From da912f7021fe9224f90d2bc2036637424d3257b2 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Tue, 18 Sep 2018 13:29:18 +0100 Subject: [PATCH 05/15] gvariant: Limit GVariant strings to G_MAXSSIZE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When validating a string to see if it’s valid UTF-8, we pass a gsize to g_utf8_validate(), which only takes a gssize. For large gsize values, this will result in the gssize actually being negative, which will change g_utf8_validate()’s behaviour to stop at the first nul byte. That would allow subsequent nul bytes through the string validator, against its documented behaviour. Add a test case. oss-fuzz#10319 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- glib/gvariant-serialiser.c | 3 ++- glib/tests/gvariant.c | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index 48405c0..d29a1e7 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -1643,6 +1643,7 @@ g_variant_serialiser_is_string (gconstpointer data, const gchar *expected_end; const gchar *end; + /* Strings must end with a nul terminator. */ if (size == 0) return FALSE; @@ -1651,7 +1652,7 @@ g_variant_serialiser_is_string (gconstpointer data, if (*expected_end != '\0') return FALSE; - g_utf8_validate (data, size, &end); + g_utf8_validate_len (data, size, &end); return end == expected_end; } diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 39c680f..d00001e 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4665,6 +4665,30 @@ test_normal_checking_tuple_offsets (void) g_variant_unref (variant); } +/* Test that an empty object path is normalised successfully to the base object + * path, ‘/’. */ +static void +test_normal_checking_empty_object_path (void) +{ + const guint8 data[] = { + 0x20, 0x20, 0x00, 0x00, 0x00, 0x00, + '(', 'h', '(', 'a', 'i', 'a', 'b', 'i', 'o', ')', ')', + }; + gsize size = sizeof (data); + GVariant *variant = NULL; + GVariant *normal_variant = NULL; + + variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, data, size, + FALSE, NULL, NULL); + g_assert_nonnull (variant); + + normal_variant = g_variant_get_normal_form (variant); + g_assert_nonnull (normal_variant); + + g_variant_unref (normal_variant); + g_variant_unref (variant); +} + int main (int argc, char **argv) { @@ -4734,6 +4758,8 @@ main (int argc, char **argv) test_normal_checking_array_offsets); g_test_add_func ("/gvariant/normal-checking/tuple-offsets", test_normal_checking_tuple_offsets); + g_test_add_func ("/gvariant/normal-checking/empty-object-path", + test_normal_checking_empty_object_path); g_test_add_func ("/gvariant/recursion-limits/variant-in-variant", test_recursion_limits_variant_in_variant); -- 2.16.3 From bc6e9f07150e6ccd62ea39f7f024fb5665753fa2 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 16 Aug 2018 21:33:52 +0100 Subject: [PATCH 06/15] gdbusmessage: Validate type of message header signature field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parsing a D-Bus message with the signature field in the message header of type other than ‘g’ (GVariant type signature) would cause a critical warning. Instead, we should return a runtime error. Includes a test. oss-fuzz#9825 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/gdbusmessage.c | 19 ++++++++++++++ gio/tests/gdbus-serialization.c | 56 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index d9d8f37..d967586 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -2109,6 +2109,15 @@ g_dbus_message_new_from_blob (guchar *blob, const gchar *signature_str; gsize signature_str_len; + if (!g_variant_is_of_type (signature, G_VARIANT_TYPE_SIGNATURE)) + { + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Signature header found but is not of type signature")); + goto out; + } + signature_str = g_variant_get_string (signature, &signature_str_len); /* signature but no body */ @@ -2689,6 +2698,16 @@ g_dbus_message_to_blob (GDBusMessage *message, body_start_offset = mbuf.valid_len; signature = g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE); + + if (signature != NULL && !g_variant_is_of_type (signature, G_VARIANT_TYPE_SIGNATURE)) + { + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Signature header found but is not of type signature")); + goto out; + } + signature_str = NULL; if (signature != NULL) signature_str = g_variant_get_string (signature, NULL); diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index a405189..416c10a 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -871,6 +871,20 @@ message_serialize_header_checks (void) g_assert (blob == NULL); g_object_unref (message); + /* + * check that we can't serialize messages with SIGNATURE set to a non-signature-typed value + */ + message = g_dbus_message_new_signal ("/the/path", "The.Interface", "TheMember"); + g_dbus_message_set_header (message, G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, g_variant_new_boolean (FALSE)); + blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); + + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert_cmpstr (error->message, ==, "Signature header found but is not of type signature"); + g_assert_null (blob); + + g_clear_error (&error); + g_clear_object (&message); + /* * check we can't serialize signal messages with INTERFACE, PATH or MEMBER unset / set to reserved value */ @@ -1081,6 +1095,46 @@ test_double_array (void) /* ---------------------------------------------------------------------------------------------------- */ +/* Test that an invalid header in a D-Bus message (specifically, with a type + * which doesn’t match what’s expected for the given header) is gracefully + * handled with an error rather than a crash. + * The set of bytes here come directly from fuzzer output. */ +static void +test_message_parse_non_signature_header (void) +{ + const guint8 data[] = { + 0x6c, /* byte order */ + 0x04, /* message type */ + 0x0f, /* message flags */ + 0x01, /* major protocol version */ + 0x00, 0x00, 0x00, 0x00, /* body length */ + 0x00, 0x00, 0x00, 0xbc, /* message serial */ + /* a{yv} of header fields: + * (things start to be invalid below here) */ + 0x02, 0x00, 0x00, 0x00, /* array length (in bytes) */ + G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, /* array key */ + /* GVariant array value: */ + 0x04, /* signature length */ + 'd', 0x00, 0x00, 'F', /* signature (invalid) */ + 0x00, /* nul terminator */ + /* (GVariant array value payload missing) */ + /* (message body length missing) */ + }; + gsize size = sizeof (data); + GDBusMessage *message = NULL; + GError *local_error = NULL; + + message = g_dbus_message_new_from_blob ((guchar *) data, size, + G_DBUS_CAPABILITY_FLAGS_NONE, + &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert_null (message); + + g_clear_error (&local_error); +} + +/* ---------------------------------------------------------------------------------------------------- */ + int main (int argc, char *argv[]) @@ -1099,6 +1153,8 @@ main (int argc, message_parse_empty_arrays_of_arrays); g_test_add_func ("/gdbus/message-serialize/double-array", test_double_array); + g_test_add_func ("/gdbus/message-parse/non-signature-header", + test_message_parse_non_signature_header); return g_test_run(); } -- 2.16.3 From 332aacbb4e7147155fe22d671befddde8aab8a88 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 16 Aug 2018 21:35:31 +0100 Subject: [PATCH 07/15] gdbusmessage: Improve documentation for g_dbus_message_get_header() The caller is responsible for checking the type of the returned GVariant. Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/gdbusmessage.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index d967586..e283a8c 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -981,7 +981,10 @@ g_dbus_message_set_serial (GDBusMessage *message, * * Gets a header field on @message. * - * Returns: A #GVariant with the value if the header was found, %NULL + * The caller is responsible for checking the type of the returned #GVariant + * matches what is expected. + * + * Returns: (transfer none) (nullable): A #GVariant with the value if the header was found, %NULL * otherwise. Do not free, it is owned by @message. * * Since: 2.26 -- 2.16.3 From 6efc913e4e2e4351609ce061f6640801dff605cb Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Tue, 18 Sep 2018 14:48:43 +0100 Subject: [PATCH 08/15] gdbusmessage: Clarify error returns for g_dbus_message_new_from_blob() Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/gdbusmessage.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index e283a8c..462b402 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -2000,6 +2000,9 @@ g_dbus_message_bytes_needed (guchar *blob, * order that the message was in can be retrieved using * g_dbus_message_get_byte_order(). * + * If the @blob cannot be parsed, contains invalid fields, or contains invalid + * headers, %G_IO_ERROR_INVALID_ARGUMENT will be returned. + * * Returns: A new #GDBusMessage or %NULL if @error is set. Free with * g_object_unref(). * -- 2.16.3 From 9be360e0e3c6d0084cb085651debc18d193bc822 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Tue, 18 Sep 2018 14:57:51 +0100 Subject: [PATCH 09/15] gdbusmessage: Fix a typo in a documentation comment Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/gdbusmessage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index 462b402..1d44410 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -1923,7 +1923,7 @@ parse_value_from_blob (GMemoryBuffer *buf, /** * g_dbus_message_bytes_needed: - * @blob: (array length=blob_len) (element-type guint8): A blob represent a binary D-Bus message. + * @blob: (array length=blob_len) (element-type guint8): A blob representing a binary D-Bus message. * @blob_len: The length of @blob (must be at least 16). * @error: Return location for error or %NULL. * -- 2.16.3 From d08d73055d0188ea302890747f6f0150a644357e Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Tue, 18 Sep 2018 15:17:44 +0100 Subject: [PATCH 10/15] gdbusmessage: Check for valid GVariantType when parsing a variant blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code was checking whether the signature provided by the blob was a valid D-Bus signature — but that’s a superset of a valid GVariant type string, since a D-Bus signature is zero or more complete types. A GVariant type string is exactly one complete type. This meant that a D-Bus message with a header field containing a variant with an empty type signature (for example) could cause a critical warning in the code parsing it. Fix that by checking whether the string is a valid type string too. Unit test included. oss-fuzz#9810 Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/gdbusmessage.c | 5 ++- gio/tests/gdbus-serialization.c | 89 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index 1d44410..832690b 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -1838,8 +1838,11 @@ parse_value_from_blob (GMemoryBuffer *buf, sig = read_string (buf, (gsize) siglen, &local_error); if (sig == NULL) goto fail; - if (!g_variant_is_signature (sig)) + if (!g_variant_is_signature (sig) || + !g_variant_type_string_is_valid (sig)) { + /* A D-Bus signature can contain zero or more complete types, + * but a GVariant has to be exactly one complete type. */ g_set_error (&local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index 416c10a..ebac2d3 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -1103,7 +1103,7 @@ static void test_message_parse_non_signature_header (void) { const guint8 data[] = { - 0x6c, /* byte order */ + 'l', /* little-endian byte order */ 0x04, /* message type */ 0x0f, /* message flags */ 0x01, /* major protocol version */ @@ -1113,11 +1113,90 @@ test_message_parse_non_signature_header (void) * (things start to be invalid below here) */ 0x02, 0x00, 0x00, 0x00, /* array length (in bytes) */ G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, /* array key */ - /* GVariant array value: */ + /* Variant array value: */ 0x04, /* signature length */ 'd', 0x00, 0x00, 'F', /* signature (invalid) */ 0x00, /* nul terminator */ - /* (GVariant array value payload missing) */ + /* (Variant array value payload missing) */ + /* (message body length missing) */ + }; + gsize size = sizeof (data); + GDBusMessage *message = NULL; + GError *local_error = NULL; + + message = g_dbus_message_new_from_blob ((guchar *) data, size, + G_DBUS_CAPABILITY_FLAGS_NONE, + &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert_null (message); + + g_clear_error (&local_error); +} + +/* ---------------------------------------------------------------------------------------------------- */ + +/* Test that an invalid header in a D-Bus message (specifically, containing a + * variant with an empty type signature) is gracefully handled with an error + * rather than a crash. The set of bytes here come directly from fuzzer + * output. */ +static void +test_message_parse_empty_signature_header (void) +{ + const guint8 data[] = { + 'l', /* little-endian byte order */ + 0x20, /* message type */ + 0x20, /* message flags */ + 0x01, /* major protocol version */ + 0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ + 0x20, 0x20, 0x20, 0x20, /* message serial */ + /* a{yv} of header fields: + * (things start to be even more invalid below here) */ + 0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */ + 0x20, /* array key */ + /* Variant array value: */ + 0x00, /* signature length */ + 0x00, /* nul terminator */ + /* (Variant array value payload missing) */ + /* (message body length missing) */ + }; + gsize size = sizeof (data); + GDBusMessage *message = NULL; + GError *local_error = NULL; + + message = g_dbus_message_new_from_blob ((guchar *) data, size, + G_DBUS_CAPABILITY_FLAGS_NONE, + &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert_null (message); + + g_clear_error (&local_error); +} + +/* ---------------------------------------------------------------------------------------------------- */ + +/* Test that an invalid header in a D-Bus message (specifically, containing a + * variant with a type signature containing multiple complete types) is + * gracefully handled with an error rather than a crash. The set of bytes here + * come directly from fuzzer output. */ +static void +test_message_parse_multiple_signature_header (void) +{ + const guint8 data[] = { + 'l', /* little-endian byte order */ + 0x20, /* message type */ + 0x20, /* message flags */ + 0x01, /* major protocol version */ + 0x20, 0x20, 0x20, 0x00, /* body length (invalid) */ + 0x20, 0x20, 0x20, 0x20, /* message serial */ + /* a{yv} of header fields: + * (things start to be even more invalid below here) */ + 0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */ + 0x20, /* array key */ + /* Variant array value: */ + 0x02, /* signature length */ + 'b', 'b', /* two complete types */ + 0x00, /* nul terminator */ + /* (Variant array value payload missing) */ /* (message body length missing) */ }; gsize size = sizeof (data); @@ -1155,6 +1234,10 @@ main (int argc, g_test_add_func ("/gdbus/message-serialize/double-array", test_double_array); g_test_add_func ("/gdbus/message-parse/non-signature-header", test_message_parse_non_signature_header); + g_test_add_func ("/gdbus/message-parse/empty-signature-header", + test_message_parse_empty_signature_header); + g_test_add_func ("/gdbus/message-parse/multiple-signature-header", + test_message_parse_multiple_signature_header); return g_test_run(); } -- 2.16.3 From e2a3e6a59c26c2f63347194aee264c7a40db8265 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Tue, 18 Sep 2018 15:21:51 +0100 Subject: [PATCH 11/15] gvariant: Clarify internal documentation about GVariant type strings Signed-off-by: Philip Withnall <withnall@endlessm.com> --- glib/gvariant-serialiser.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c index d29a1e7..3b8bd40 100644 --- a/glib/gvariant-serialiser.c +++ b/glib/gvariant-serialiser.c @@ -1717,7 +1717,9 @@ g_variant_serialiser_is_object_path (gconstpointer data, * Performs the checks for being a valid string. * * Also, ensures that @data is a valid D-Bus type signature, as per the - * D-Bus specification. + * D-Bus specification. Note that this means the empty string is valid, as the + * D-Bus specification defines a signature as “zero or more single complete + * types”. */ gboolean g_variant_serialiser_is_signature (gconstpointer data, -- 2.16.3 From f47bc2f2dce0e9cb2a116870dc29c3abdd1ae384 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 20 Sep 2018 17:41:10 +0100 Subject: [PATCH 12/15] tests: Tidy up GError handling in gdbus-serialization test This introduces no functional changes; just a bit of code tidying. Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/tests/gdbus-serialization.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index ebac2d3..68da58e 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -855,7 +855,7 @@ message_serialize_header_checks (void) { GDBusMessage *message; GDBusMessage *reply; - GError *error; + GError *error = NULL; guchar *blob; gsize blob_size; @@ -863,11 +863,10 @@ message_serialize_header_checks (void) * check we can't serialize messages with INVALID type */ message = g_dbus_message_new (); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: type is INVALID"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); g_object_unref (message); @@ -892,49 +891,44 @@ message_serialize_header_checks (void) /* ----- */ /* interface NULL => error */ g_dbus_message_set_interface (message, NULL); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* interface reserved value => error */ g_dbus_message_set_interface (message, "org.freedesktop.DBus.Local"); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: The INTERFACE header field is using the reserved value org.freedesktop.DBus.Local"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset interface */ g_dbus_message_set_interface (message, "The.Interface"); /* ----- */ /* path NULL => error */ g_dbus_message_set_path (message, NULL); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* path reserved value => error */ g_dbus_message_set_path (message, "/org/freedesktop/DBus/Local"); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: The PATH header field is using the reserved value /org/freedesktop/DBus/Local"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset path */ g_dbus_message_set_path (message, "/the/path"); /* ----- */ /* member NULL => error */ g_dbus_message_set_member (message, NULL); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset member */ g_dbus_message_set_member (message, "TheMember"); @@ -949,22 +943,20 @@ message_serialize_header_checks (void) /* ----- */ /* path NULL => error */ g_dbus_message_set_path (message, NULL); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset path */ g_dbus_message_set_path (message, "/the/path"); /* ----- */ /* member NULL => error */ g_dbus_message_set_member (message, NULL); - error = NULL; blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset member */ g_dbus_message_set_member (message, "TheMember"); @@ -981,11 +973,10 @@ message_serialize_header_checks (void) reply = g_dbus_message_new_method_reply (message); g_assert_cmpint (g_dbus_message_get_reply_serial (reply), ==, 42); g_dbus_message_set_header (reply, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL, NULL); - error = NULL; blob = g_dbus_message_to_blob (reply, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_RETURN message: REPLY_SERIAL header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); g_object_unref (reply); /* method error - first nuke ERROR_NAME, then REPLY_SERIAL */ @@ -993,21 +984,19 @@ message_serialize_header_checks (void) g_assert_cmpint (g_dbus_message_get_reply_serial (reply), ==, 42); /* nuke ERROR_NAME */ g_dbus_message_set_error_name (reply, NULL); - error = NULL; blob = g_dbus_message_to_blob (reply, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); /* reset ERROR_NAME */ g_dbus_message_set_error_name (reply, "Some.Error.Name"); /* nuke REPLY_SERIAL */ g_dbus_message_set_header (reply, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL, NULL); - error = NULL; blob = g_dbus_message_to_blob (reply, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing"); - g_error_free (error); + g_clear_error (&error); g_assert (blob == NULL); g_object_unref (reply); g_object_unref (message); -- 2.16.3 From 92e59952e75105fc16412d667f1f691ecd54e112 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 20 Sep 2018 17:42:05 +0100 Subject: [PATCH 13/15] tests: Use g_assert_null() in gdbus-serialization test This introduces no real functional changes (except when compiling with G_DISABLE_ASSERT, in which case it fixes the test). Mostly just a code cleanup. Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/tests/gdbus-serialization.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index 68da58e..ddf48a4 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -867,7 +867,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: type is INVALID"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); g_object_unref (message); /* @@ -895,14 +895,14 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* interface reserved value => error */ g_dbus_message_set_interface (message, "org.freedesktop.DBus.Local"); blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: The INTERFACE header field is using the reserved value org.freedesktop.DBus.Local"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset interface */ g_dbus_message_set_interface (message, "The.Interface"); /* ----- */ @@ -912,14 +912,14 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* path reserved value => error */ g_dbus_message_set_path (message, "/org/freedesktop/DBus/Local"); blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: The PATH header field is using the reserved value /org/freedesktop/DBus/Local"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset path */ g_dbus_message_set_path (message, "/the/path"); /* ----- */ @@ -929,7 +929,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset member */ g_dbus_message_set_member (message, "TheMember"); /* ----- */ @@ -947,7 +947,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset path */ g_dbus_message_set_path (message, "/the/path"); /* ----- */ @@ -957,7 +957,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset member */ g_dbus_message_set_member (message, "TheMember"); /* ----- */ @@ -977,7 +977,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: METHOD_RETURN message: REPLY_SERIAL header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); g_object_unref (reply); /* method error - first nuke ERROR_NAME, then REPLY_SERIAL */ reply = g_dbus_message_new_method_error (message, "Some.Error.Name", "the message"); @@ -988,7 +988,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); /* reset ERROR_NAME */ g_dbus_message_set_error_name (reply, "Some.Error.Name"); /* nuke REPLY_SERIAL */ @@ -997,7 +997,7 @@ message_serialize_header_checks (void) g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_cmpstr (error->message, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing"); g_clear_error (&error); - g_assert (blob == NULL); + g_assert_null (blob); g_object_unref (reply); g_object_unref (message); } -- 2.16.3 From ed43aed0285ae2fd3ac27e7908c554be2ee578a1 Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Mon, 1 Oct 2018 19:52:14 +0100 Subject: [PATCH 14/15] gutf8: Add a g_utf8_validate_len() function This is a variant of g_utf8_validate() which requires the length to be specified, thereby allowing string lengths up to G_MAXSIZE rather than just G_MAXSSIZE. Signed-off-by: Philip Withnall <withnall@endlessm.com> --- docs/reference/glib/glib-sections.txt | 1 + glib/gunicode.h | 4 ++++ glib/gutf8.c | 42 ++++++++++++++++++++++++++++++----- glib/tests/utf8-validate.c | 15 +++++++++---- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 23abefa..84599df 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -2899,6 +2899,7 @@ g_utf8_strrchr g_utf8_strreverse g_utf8_substring g_utf8_validate +g_utf8_validate_len <SUBSECTION> g_utf8_strup diff --git a/glib/gunicode.h b/glib/gunicode.h index 5f04536..79778f7 100644 --- a/glib/gunicode.h +++ b/glib/gunicode.h @@ -804,6 +804,10 @@ GLIB_AVAILABLE_IN_ALL gboolean g_utf8_validate (const gchar *str, gssize max_len, const gchar **end); +GLIB_AVAILABLE_IN_2_48 +gboolean g_utf8_validate_len (const gchar *str, + gsize max_len, + const gchar **end); GLIB_AVAILABLE_IN_ALL gchar *g_utf8_strup (const gchar *str, diff --git a/glib/gutf8.c b/glib/gutf8.c index eae10da..bc08077 100644 --- a/glib/gutf8.c +++ b/glib/gutf8.c @@ -1655,16 +1655,48 @@ g_utf8_validate (const char *str, { const gchar *p; - if (max_len < 0) - p = fast_validate (str); + if (max_len >= 0) + return g_utf8_validate_len (str, max_len, end); + + p = fast_validate (str); + + if (end) + *end = p; + + if (*p != '\0') + return FALSE; else - p = fast_validate_len (str, max_len); + return TRUE; +} + +/** + * g_utf8_validate_len: + * @str: (array length=max_len) (element-type guint8): a pointer to character data + * @max_len: max bytes to validate + * @end: (out) (optional) (transfer none): return location for end of valid data + * + * Validates UTF-8 encoded text. + * + * As with g_utf8_validate(), but @max_len must be set, and hence this function + * will always return %FALSE if any of the bytes of @str are nul. + * + * Returns: %TRUE if the text was valid UTF-8 + * Since: 2.60 + */ +gboolean +g_utf8_validate_len (const char *str, + gsize max_len, + const gchar **end) + +{ + const gchar *p; + + p = fast_validate_len (str, max_len); if (end) *end = p; - if ((max_len >= 0 && p != str + max_len) || - (max_len < 0 && *p != '\0')) + if (p != str + max_len) return FALSE; else return TRUE; diff --git a/glib/tests/utf8-validate.c b/glib/tests/utf8-validate.c index 8c97b40..786400d 100644 --- a/glib/tests/utf8-validate.c +++ b/glib/tests/utf8-validate.c @@ -280,15 +280,22 @@ do_test (gconstpointer d) result = g_utf8_validate (test->text, test->max_len, &end); - g_assert (result == test->valid); - g_assert (end - test->text == test->offset); + g_assert_true (result == test->valid); + g_assert_cmpint (end - test->text, ==, test->offset); if (test->max_len < 0) { result = g_utf8_validate (test->text, strlen (test->text), &end); - g_assert (result == test->valid); - g_assert (end - test->text == test->offset); + g_assert_true (result == test->valid); + g_assert_cmpint (end - test->text, ==, test->offset); + } + else + { + result = g_utf8_validate_len (test->text, test->max_len, &end); + + g_assert_true (result == test->valid); + g_assert_cmpint (end - test->text, ==, test->offset); } } -- 2.16.3 From 11f83097a0fe6df7b72c6d117fee35e17e02096c Mon Sep 17 00:00:00 2001 From: Philip Withnall <withnall@endlessm.com> Date: Thu, 4 Oct 2018 13:22:13 +0100 Subject: [PATCH 15/15] glib: Port various callers to use g_utf8_validate_len() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These were callers which explicitly specified the string length to g_utf8_validate(), when it couldn’t be negative, and hence should be able to unconditionally benefit from the increased string handling length. At least one call site would have previously silently changed behaviour if called with strings longer than G_MAXSSIZE in length. Another call site was passing strlen(string) to g_utf8_validate(), which seems pointless: just pass -1 instead, and let g_utf8_validate() calculate the string length. Its behaviour on embedded nul bytes wouldn’t change, as strlen() stops at the first one. Signed-off-by: Philip Withnall <withnall@endlessm.com> --- gio/glocalfileinfo.c | 4 ++-- glib/giochannel.c | 2 +- glib/gmarkup.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gio/glocalfileinfo.c b/gio/glocalfileinfo.c index 620320f..48c8f8d 100644 --- a/gio/glocalfileinfo.c +++ b/gio/glocalfileinfo.c @@ -1018,7 +1018,7 @@ make_valid_utf8 (const char *name) { GString *string; const gchar *remainder, *invalid; - gint remaining_bytes, valid_bytes; + gsize remaining_bytes, valid_bytes; string = NULL; remainder = name; @@ -1026,7 +1026,7 @@ make_valid_utf8 (const char *name) while (remaining_bytes != 0) { - if (g_utf8_validate (remainder, remaining_bytes, &invalid)) + if (g_utf8_validate_len (remainder, remaining_bytes, &invalid)) break; valid_bytes = invalid - remainder; diff --git a/glib/giochannel.c b/glib/giochannel.c index 1f01336..b9ce9c1 100644 --- a/glib/giochannel.c +++ b/glib/giochannel.c @@ -2322,7 +2322,7 @@ reconvert: /* UTF-8, just validate, emulate g_iconv */ - if (!g_utf8_validate (from_buf, try_len, &badchar)) + if (!g_utf8_validate_len (from_buf, try_len, &badchar)) { gunichar try_char; gsize incomplete_len = from_buf + try_len - badchar; diff --git a/glib/gmarkup.c b/glib/gmarkup.c index 7103d48..a0a3f73 100644 --- a/glib/gmarkup.c +++ b/glib/gmarkup.c @@ -456,7 +456,7 @@ slow_name_validate (GMarkupParseContext *context, { const gchar *p = name; - if (!g_utf8_validate (name, strlen (name), NULL)) + if (!g_utf8_validate (name, -1, NULL)) { set_error (context, error, G_MARKUP_ERROR_BAD_UTF8, _("Invalid UTF-8 encoded text in name - not valid '%s'"), name); @@ -539,7 +539,7 @@ text_validate (GMarkupParseContext *context, gint len, GError **error) { - if (!g_utf8_validate (p, len, NULL)) + if (!g_utf8_validate_len (p, len, NULL)) { set_error (context, error, G_MARKUP_ERROR_BAD_UTF8, _("Invalid UTF-8 encoded text in name - not valid '%s'"), p); -- 2.16.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
