mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2024-11-10 11:26:16 +01:00
gvariant: Cut allocs of default values for children of non-normal arrays
This improves a slow case in `g_variant_get_normal_form()` where allocating many identical default values for the children of a variable-sized array which has a malformed offset table would take a lot of time. The fix is to make all child values after the first invalid one be references to the default value emitted for the first invalid one, rather than identical new `GVariant`s. In particular, this fixes a case where an attacker could create an array of length L of very large tuples of size T each, corrupt the offset table so they don’t have to specify the array content, and then induce `g_variant_get_normal_form()` into allocating L×T default values from an input which is significantly smaller than L×T in length. A pre-existing workaround for this issue is for code to call `g_variant_is_normal_form()` before calling `g_variant_get_normal_form()`, and to skip the latter call if the former returns false. This commit improves the behaviour in the case that `g_variant_get_normal_form()` is called anyway. This fix changes the time to run the `fuzz_variant_binary` test on the testcase from oss-fuzz#19777 from >60s (before being terminated) with 2.3GB of memory usage and 580k page faults; to 32s, 8.3MB of memory usage and 1500 page faults (as measured by `time -v`). Signed-off-by: Philip Withnall <pwithnall@endlessos.org> Fixes: #2540 oss-fuzz#19777
This commit is contained in:
parent
168f9b42e5
commit
c2dc74e2ec
@ -5857,7 +5857,6 @@ g_variant_deep_copy (GVariant *value)
|
||||
switch (g_variant_classify (value))
|
||||
{
|
||||
case G_VARIANT_CLASS_MAYBE:
|
||||
case G_VARIANT_CLASS_ARRAY:
|
||||
case G_VARIANT_CLASS_TUPLE:
|
||||
case G_VARIANT_CLASS_DICT_ENTRY:
|
||||
case G_VARIANT_CLASS_VARIANT:
|
||||
@ -5877,6 +5876,71 @@ g_variant_deep_copy (GVariant *value)
|
||||
return g_variant_builder_end (&builder);
|
||||
}
|
||||
|
||||
case G_VARIANT_CLASS_ARRAY:
|
||||
{
|
||||
GVariantBuilder builder;
|
||||
gsize i, n_children;
|
||||
GVariant *first_invalid_child_deep_copy = NULL;
|
||||
|
||||
/* Arrays are in theory treated the same as maybes, tuples, dict entries
|
||||
* and variants, and could be another case in the above block of code.
|
||||
*
|
||||
* However, they have the property that when dealing with non-normal
|
||||
* data (which is the only time g_variant_deep_copy() is currently
|
||||
* called) in a variable-sized array, the code above can easily end up
|
||||
* creating many default child values in order to return an array which
|
||||
* is of the right length and type, but without containing non-normal
|
||||
* data. This can happen if the offset table for the array is malformed.
|
||||
*
|
||||
* In this case, the code above would end up allocating the same default
|
||||
* value for each one of the child indexes beyond the first malformed
|
||||
* entry in the offset table. This can end up being a lot of identical
|
||||
* allocations of default values, particularly if the non-normal array
|
||||
* is crafted maliciously.
|
||||
*
|
||||
* Avoid that problem by returning a new reference to the same default
|
||||
* value for every child after the first invalid one. This results in
|
||||
* returning an equivalent array, in normal form and trusted — but with
|
||||
* significantly fewer memory allocations.
|
||||
*
|
||||
* See https://gitlab.gnome.org/GNOME/glib/-/issues/2540 */
|
||||
|
||||
g_variant_builder_init (&builder, g_variant_get_type (value));
|
||||
|
||||
for (i = 0, n_children = g_variant_n_children (value); i < n_children; i++)
|
||||
{
|
||||
/* Try maybe_get_child_value() first; if it returns NULL, this child
|
||||
* is non-normal. get_child_value() would have constructed and
|
||||
* returned a default value in that case. */
|
||||
GVariant *child = g_variant_maybe_get_child_value (value, i);
|
||||
|
||||
if (child != NULL)
|
||||
{
|
||||
/* Non-normal children may not always be contiguous, as they may
|
||||
* be non-normal for reasons other than invalid offset table
|
||||
* entries. As they are all the same type, they will all have
|
||||
* the same default value though, so keep that around. */
|
||||
g_variant_builder_add_value (&builder, g_variant_deep_copy (child));
|
||||
}
|
||||
else if (child == NULL && first_invalid_child_deep_copy != NULL)
|
||||
{
|
||||
g_variant_builder_add_value (&builder, first_invalid_child_deep_copy);
|
||||
}
|
||||
else if (child == NULL)
|
||||
{
|
||||
child = g_variant_get_child_value (value, i);
|
||||
first_invalid_child_deep_copy = g_variant_ref_sink (g_variant_deep_copy (child));
|
||||
g_variant_builder_add_value (&builder, first_invalid_child_deep_copy);
|
||||
}
|
||||
|
||||
g_clear_pointer (&child, g_variant_unref);
|
||||
}
|
||||
|
||||
g_clear_pointer (&first_invalid_child_deep_copy, g_variant_unref);
|
||||
|
||||
return g_variant_builder_end (&builder);
|
||||
}
|
||||
|
||||
case G_VARIANT_CLASS_BOOLEAN:
|
||||
return g_variant_new_boolean (g_variant_get_boolean (value));
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user