File 0003-gvariant-Check-array-offsets-against-seria... of Package glib2

Overview Repositories Revisions Requests Users Attributes Meta

File 0003-gvariant-Check-array-offsets-against-serialised-data.patch of Package glib2

From a98af8abae7eea0f9392b1052b67ab09a6d398f0 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      | 76 +++++++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c
index fe0bcf0aa..aa71d3c1c 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 0d996ae72..0992b0a8f 100644
--- a/glib/tests/gvariant.c
+++ b/glib/tests/gvariant.c
@@ -4737,9 +4737,7 @@ test_recursion_limits_variant_in_variant (void)
  * worth testing, because neither the deeply nested array, nor the variant,
  * have a static #GVariantType which is too deep — only when nested together do
  * they become too deep. */
-static void
-test_recursion_limits_array_in_variant (void)
-{
+static void test_recursion_limits_array_in_variant(void) {
   GVariant *child_variant = NULL;
   GVariant *wrapper_variant = NULL;
   gsize i;
@@ -4748,41 +4746,65 @@ test_recursion_limits_array_in_variant (void)
 
   /* Construct a hierarchy of arrays, containing a single string. This is just
    * below the maximum recursion level, all in a single definite type. */
-  child_variant = g_variant_new_string ("hello");
+  child_variant = g_variant_new_string("hello");
 
   for (i = 0; i < G_VARIANT_MAX_RECURSION_DEPTH - 1; i++)
-    child_variant = g_variant_new_array (NULL, &child_variant, 1);
+    child_variant = g_variant_new_array(NULL, &child_variant, 1);
 
   /* Serialise and deserialise it as untrusted data, to force normalisation. */
-  bytes = g_variant_get_data_as_bytes (child_variant);
-  deserialised_variant = g_variant_new_from_bytes (g_variant_get_type (child_variant),
-                                                   bytes, FALSE);
-  g_assert_nonnull (deserialised_variant);
-  g_assert_true (g_variant_is_normal_form (deserialised_variant));
+  bytes = g_variant_get_data_as_bytes(child_variant);
+  deserialised_variant =
+      g_variant_new_from_bytes(g_variant_get_type(child_variant), bytes, FALSE);
+  g_assert_nonnull(deserialised_variant);
+  g_assert_true(g_variant_is_normal_form(deserialised_variant));
 
-  g_bytes_unref (bytes);
-  g_variant_unref (deserialised_variant);
+  g_bytes_unref(bytes);
+  g_variant_unref(deserialised_variant);
 
   /* Wrap it in a variant. Normalisation should now fail. */
-  wrapper_variant = g_variant_new_variant (g_steal_pointer (&child_variant));
+  wrapper_variant = g_variant_new_variant(g_steal_pointer(&child_variant));
 
-  bytes = g_variant_get_data_as_bytes (wrapper_variant);
-  deserialised_variant = g_variant_new_from_bytes (G_VARIANT_TYPE_VARIANT,
-                                                   bytes, FALSE);
-  g_assert_nonnull (deserialised_variant);
-  g_assert_false (g_variant_is_normal_form (deserialised_variant));
+  bytes = g_variant_get_data_as_bytes(wrapper_variant);
+  deserialised_variant =
+      g_variant_new_from_bytes(G_VARIANT_TYPE_VARIANT, bytes, FALSE);
+  g_assert_nonnull(deserialised_variant);
+  g_assert_false(g_variant_is_normal_form(deserialised_variant));
 
-  g_variant_unref (deserialised_variant);
+  g_variant_unref(deserialised_variant);
 
   /* Deserialise it again, but trusted this time. This should succeed. */
-  deserialised_variant = g_variant_new_from_bytes (G_VARIANT_TYPE_VARIANT,
-                                                   bytes, TRUE);
-  g_assert_nonnull (deserialised_variant);
-  g_assert_true (g_variant_is_normal_form (deserialised_variant));
+  deserialised_variant =
+      g_variant_new_from_bytes(G_VARIANT_TYPE_VARIANT, bytes, TRUE);
+  g_assert_nonnull(deserialised_variant);
+  g_assert_true(g_variant_is_normal_form(deserialised_variant));
+
+  g_bytes_unref(bytes);
+  g_variant_unref(deserialised_variant);
+  g_variant_unref(wrapper_variant);
+}
 
-  g_bytes_unref (bytes);
-  g_variant_unref (deserialised_variant);
-  g_variant_unref (wrapper_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
@@ -4853,6 +4875,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.41.0

Places

File 0003-gvariant-Check-array-offsets-against-serialised-data.patch of Package glib2

Places