1From 1deacdd4e8e35a5cf1417918ca4f6b0afa6409b1 Mon Sep 17 00:00:00 2001 2From: William Manley <will@stb-tester.com> 3Date: Tue, 23 Jun 2020 22:59:58 +0100 4Subject: [PATCH 01/18] gvariant-core: Consolidate construction of 5 `GVariantSerialised` 6 7So I only need to change it in one place. 8 9This introduces no functional changes. 10 11Helps: #2121 12--- 13 glib/gvariant-core.c | 49 ++++++++++++++++++++++---------------------- 14 1 file changed, 25 insertions(+), 24 deletions(-) 15 16diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 17index 89ea54c013..d5d78da88b 100644 18--- a/glib/gvariant-core.c 19+++ b/glib/gvariant-core.c 20@@ -351,6 +351,27 @@ g_variant_ensure_size (GVariant *value) 21 } 22 } 23 24+/* < private > 25+ * g_variant_to_serialised: 26+ * @value: a #GVariant 27+ * 28+ * Gets a GVariantSerialised for a GVariant in state STATE_SERIALISED. 29+ */ 30+inline static GVariantSerialised 31+g_variant_to_serialised (GVariant *value) 32+{ 33+ g_assert (value->state & STATE_SERIALISED); 34+ { 35+ GVariantSerialised serialised = { 36+ value->type_info, 37+ (gpointer) value->contents.serialised.data, 38+ value->size, 39+ value->depth, 40+ }; 41+ return serialised; 42+ } 43+} 44+ 45 /* < private > 46 * g_variant_serialise: 47 * @value: a #GVariant 48@@ -1009,16 +1030,8 @@ g_variant_n_children (GVariant *value) 49 g_variant_lock (value); 50 51 if (value->state & STATE_SERIALISED) 52- { 53- GVariantSerialised serialised = { 54- value->type_info, 55- (gpointer) value->contents.serialised.data, 56- value->size, 57- value->depth, 58- }; 59- 60- n_children = g_variant_serialised_n_children (serialised); 61- } 62+ n_children = g_variant_serialised_n_children ( 63+ g_variant_to_serialised (value)); 64 else 65 n_children = value->contents.tree.n_children; 66 67@@ -1085,12 +1098,7 @@ g_variant_get_child_value (GVariant *value, 68 } 69 70 { 71- GVariantSerialised serialised = { 72- value->type_info, 73- (gpointer) value->contents.serialised.data, 74- value->size, 75- value->depth, 76- }; 77+ GVariantSerialised serialised = g_variant_to_serialised (value); 78 GVariantSerialised s_child; 79 GVariant *child; 80 81@@ -1203,14 +1211,7 @@ g_variant_is_normal_form (GVariant *value) 82 83 if (value->state & STATE_SERIALISED) 84 { 85- GVariantSerialised serialised = { 86- value->type_info, 87- (gpointer) value->contents.serialised.data, 88- value->size, 89- value->depth 90- }; 91- 92- if (g_variant_serialised_is_normal (serialised)) 93+ if (g_variant_serialised_is_normal (g_variant_to_serialised (value))) 94 value->state |= STATE_TRUSTED; 95 } 96 else 97-- 98GitLab 99 100 101From 446e69f5edd72deb2196dee36bbaf8056caf6948 Mon Sep 17 00:00:00 2001 102From: William Manley <will@stb-tester.com> 103Date: Thu, 25 Jun 2020 17:08:21 +0100 104Subject: [PATCH 02/18] gvariant-serialiser: Factor out functions for dealing 105 with framing offsets 106 107This introduces no functional changes. 108 109Helps: #2121 110--- 111 glib/gvariant-serialiser.c | 108 +++++++++++++++++++------------------ 112 1 file changed, 57 insertions(+), 51 deletions(-) 113 114diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 115index 6ebaec7d40..1eaa80b29f 100644 116--- a/glib/gvariant-serialiser.c 117+++ b/glib/gvariant-serialiser.c 118@@ -635,30 +635,62 @@ gvs_calculate_total_size (gsize body_size, 119 return body_size + 8 * offsets; 120 } 121 122+struct Offsets 123+{ 124+ gsize data_size; 125+ 126+ guchar *array; 127+ gsize length; 128+ guint offset_size; 129+ 130+ gboolean is_normal; 131+}; 132+ 133 static gsize 134-gvs_variable_sized_array_n_children (GVariantSerialised value) 135+gvs_offsets_get_offset_n (struct Offsets *offsets, 136+ gsize n) 137+{ 138+ return gvs_read_unaligned_le ( 139+ offsets->array + (offsets->offset_size * n), offsets->offset_size); 140+} 141+ 142+static struct Offsets 143+gvs_variable_sized_array_get_frame_offsets (GVariantSerialised value) 144 { 145+ struct Offsets out = { 0, }; 146 gsize offsets_array_size; 147- gsize offset_size; 148 gsize last_end; 149 150 if (value.size == 0) 151- return 0; 152- 153- offset_size = gvs_get_offset_size (value.size); 154+ { 155+ out.is_normal = TRUE; 156+ return out; 157+ } 158 159- last_end = gvs_read_unaligned_le (value.data + value.size - 160- offset_size, offset_size); 161+ out.offset_size = gvs_get_offset_size (value.size); 162+ last_end = gvs_read_unaligned_le (value.data + value.size - out.offset_size, 163+ out.offset_size); 164 165 if (last_end > value.size) 166- return 0; 167+ return out; /* offsets not normal */ 168 169 offsets_array_size = value.size - last_end; 170 171- if (offsets_array_size % offset_size) 172- return 0; 173+ if (offsets_array_size % out.offset_size) 174+ return out; /* offsets not normal */ 175+ 176+ out.data_size = last_end; 177+ out.array = value.data + last_end; 178+ out.length = offsets_array_size / out.offset_size; 179+ out.is_normal = TRUE; 180 181- return offsets_array_size / offset_size; 182+ return out; 183+} 184+ 185+static gsize 186+gvs_variable_sized_array_n_children (GVariantSerialised value) 187+{ 188+ return gvs_variable_sized_array_get_frame_offsets (value).length; 189 } 190 191 static GVariantSerialised 192@@ -666,8 +698,9 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, 193 gsize index_) 194 { 195 GVariantSerialised child = { 0, }; 196- gsize offset_size; 197- gsize last_end; 198+ 199+ struct Offsets offsets = gvs_variable_sized_array_get_frame_offsets (value); 200+ 201 gsize start; 202 gsize end; 203 204@@ -675,18 +708,11 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, 205 g_variant_type_info_ref (child.type_info); 206 child.depth = value.depth + 1; 207 208- offset_size = gvs_get_offset_size (value.size); 209- 210- last_end = gvs_read_unaligned_le (value.data + value.size - 211- offset_size, offset_size); 212- 213 if (index_ > 0) 214 { 215 guint alignment; 216 217- start = gvs_read_unaligned_le (value.data + last_end + 218- (offset_size * (index_ - 1)), 219- offset_size); 220+ start = gvs_offsets_get_offset_n (&offsets, index_ - 1); 221 222 g_variant_type_info_query (child.type_info, &alignment, NULL); 223 start += (-start) & alignment; 224@@ -694,11 +720,9 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, 225 else 226 start = 0; 227 228- end = gvs_read_unaligned_le (value.data + last_end + 229- (offset_size * index_), 230- offset_size); 231+ end = gvs_offsets_get_offset_n (&offsets, index_); 232 233- if (start < end && end <= value.size && end <= last_end) 234+ if (start < end && end <= value.size && end <= offsets.data_size) 235 { 236 child.data = value.data + start; 237 child.size = end - start; 238@@ -770,34 +794,16 @@ static gboolean 239 gvs_variable_sized_array_is_normal (GVariantSerialised value) 240 { 241 GVariantSerialised child = { 0, }; 242- gsize offsets_array_size; 243- guchar *offsets_array; 244- guint offset_size; 245 guint alignment; 246- gsize last_end; 247- gsize length; 248 gsize offset; 249 gsize i; 250 251- if (value.size == 0) 252- return TRUE; 253- 254- offset_size = gvs_get_offset_size (value.size); 255- last_end = gvs_read_unaligned_le (value.data + value.size - 256- offset_size, offset_size); 257+ struct Offsets offsets = gvs_variable_sized_array_get_frame_offsets (value); 258 259- if (last_end > value.size) 260+ if (!offsets.is_normal) 261 return FALSE; 262 263- offsets_array_size = value.size - last_end; 264- 265- if (offsets_array_size % offset_size) 266- return FALSE; 267- 268- offsets_array = value.data + value.size - offsets_array_size; 269- length = offsets_array_size / offset_size; 270- 271- if (length == 0) 272+ if (value.size != 0 && offsets.length == 0) 273 return FALSE; 274 275 child.type_info = g_variant_type_info_element (value.type_info); 276@@ -805,14 +811,14 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 277 child.depth = value.depth + 1; 278 offset = 0; 279 280- for (i = 0; i < length; i++) 281+ for (i = 0; i < offsets.length; i++) 282 { 283 gsize this_end; 284 285- this_end = gvs_read_unaligned_le (offsets_array + offset_size * i, 286- offset_size); 287+ this_end = gvs_read_unaligned_le (offsets.array + offsets.offset_size * i, 288+ offsets.offset_size); 289 290- if (this_end < offset || this_end > last_end) 291+ if (this_end < offset || this_end > offsets.data_size) 292 return FALSE; 293 294 while (offset & alignment) 295@@ -834,7 +840,7 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 296 offset = this_end; 297 } 298 299- g_assert (offset == last_end); 300+ g_assert (offset == offsets.data_size); 301 302 return TRUE; 303 } 304-- 305GitLab 306 307 308From 298a537d5f6783e55d87e40011ee3fd3b22b72f9 Mon Sep 17 00:00:00 2001 309From: Philip Withnall <pwithnall@endlessos.org> 310Date: Tue, 25 Oct 2022 18:05:52 +0100 311Subject: [PATCH 03/18] gvariant: Zero-initialise various GVariantSerialised 312 objects 313 314The following few commits will add a couple of new fields to 315`GVariantSerialised`, and they should be zero-filled by default. 316 317Try and pre-empt that a bit by zero-filling `GVariantSerialised` by 318default in a few places. 319 320Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 321 322Helps: #2121 323--- 324 glib/gvariant.c | 2 +- 325 glib/tests/gvariant.c | 12 ++++++------ 326 2 files changed, 7 insertions(+), 7 deletions(-) 327 328diff --git a/glib/gvariant.c b/glib/gvariant.c 329index 6863f341d6..e4c85ca636 100644 330--- a/glib/gvariant.c 331+++ b/glib/gvariant.c 332@@ -6002,7 +6002,7 @@ g_variant_byteswap (GVariant *value) 333 if (alignment) 334 /* (potentially) contains multi-byte numeric data */ 335 { 336- GVariantSerialised serialised; 337+ GVariantSerialised serialised = { 0, }; 338 GVariant *trusted; 339 GBytes *bytes; 340 341diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 342index 058d02ad22..18fa855aeb 100644 343--- a/glib/tests/gvariant.c 344+++ b/glib/tests/gvariant.c 345@@ -1440,7 +1440,7 @@ test_maybe (void) 346 347 for (flavour = 0; flavour < 8; flavour += alignment) 348 { 349- GVariantSerialised serialised; 350+ GVariantSerialised serialised = { 0, }; 351 GVariantSerialised child; 352 353 serialised.type_info = type_info; 354@@ -1564,7 +1564,7 @@ test_array (void) 355 356 for (flavour = 0; flavour < 8; flavour += alignment) 357 { 358- GVariantSerialised serialised; 359+ GVariantSerialised serialised = { 0, }; 360 361 serialised.type_info = array_info; 362 serialised.data = flavoured_malloc (needed_size, flavour); 363@@ -1728,7 +1728,7 @@ test_tuple (void) 364 365 for (flavour = 0; flavour < 8; flavour += alignment) 366 { 367- GVariantSerialised serialised; 368+ GVariantSerialised serialised = { 0, }; 369 370 serialised.type_info = type_info; 371 serialised.data = flavoured_malloc (needed_size, flavour); 372@@ -1823,7 +1823,7 @@ test_variant (void) 373 374 for (flavour = 0; flavour < 8; flavour += alignment) 375 { 376- GVariantSerialised serialised; 377+ GVariantSerialised serialised = { 0, }; 378 GVariantSerialised child; 379 380 serialised.type_info = type_info; 381@@ -2270,7 +2270,7 @@ serialise_tree (TreeInstance *tree, 382 static void 383 test_byteswap (void) 384 { 385- GVariantSerialised one, two; 386+ GVariantSerialised one = { 0, }, two = { 0, }; 387 TreeInstance *tree; 388 389 tree = tree_instance_new (NULL, 3); 390@@ -2344,7 +2344,7 @@ test_serialiser_children (void) 391 static void 392 test_fuzz (gdouble *fuzziness) 393 { 394- GVariantSerialised serialised; 395+ GVariantSerialised serialised = { 0, }; 396 TreeInstance *tree; 397 398 /* make an instance */ 399-- 400GitLab 401 402 403From ade71fb544391b2e33e1859645726bfee0d5eaaf Mon Sep 17 00:00:00 2001 404From: William Manley <will@stb-tester.com> 405Date: Mon, 29 Jun 2020 16:59:44 +0100 406Subject: [PATCH 04/18] =?UTF-8?q?gvariant:=20Don=E2=80=99t=20allow=20child?= 407 =?UTF-8?q?=20elements=20to=20overlap=20with=20each=20other?= 408MIME-Version: 1.0 409Content-Type: text/plain; charset=UTF-8 410Content-Transfer-Encoding: 8bit 411 412If different elements of a variable sized array can overlap with each 413other then we can cause a `GVariant` to normalise to a much larger type. 414 415This commit changes the behaviour of `GVariant` with non-normal form data. If 416an invalid frame offset is found all subsequent elements are given their 417default value. 418 419When retrieving an element at index `n` we scan the frame offsets up to index 420`n` and if they are not in order we return an element with the default value 421for that type. This guarantees that elements don't overlap with each 422other. We remember the offset we've scanned up to so we don't need to 423repeat this work on subsequent accesses. We skip these checks for trusted 424data. 425 426Unfortunately this makes random access of untrusted data O(n) — at least 427on first access. It doesn't affect the algorithmic complexity of accessing 428elements in order, such as when using the `GVariantIter` interface. Also: 429the cost of validation will be amortised as the `GVariant` instance is 430continued to be used. 431 432I've implemented this with 4 different functions, 1 for each element size, 433rather than looping calling `gvs_read_unaligned_le` in the hope that the 434compiler will find it easy to optimise and should produce fairly tight 435code. 436 437Fixes: #2121 438--- 439 glib/gvariant-core.c | 35 ++++++++++++++++ 440 glib/gvariant-serialiser.c | 86 ++++++++++++++++++++++++++++++++++++-- 441 glib/gvariant-serialiser.h | 9 ++++ 442 glib/tests/gvariant.c | 45 ++++++++++++++++++++ 443 4 files changed, 172 insertions(+), 3 deletions(-) 444 445diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 446index d5d78da88b..f25d472794 100644 447--- a/glib/gvariant-core.c 448+++ b/glib/gvariant-core.c 449@@ -67,6 +67,7 @@ struct _GVariant 450 { 451 GBytes *bytes; 452 gconstpointer data; 453+ gsize ordered_offsets_up_to; 454 } serialised; 455 456 struct 457@@ -164,6 +165,24 @@ struct _GVariant 458 * if .data pointed to the appropriate number of nul 459 * bytes. 460 * 461+ * .ordered_offsets_up_to: If ordered_offsets_up_to == n this means that all 462+ * the frame offsets up to and including the frame 463+ * offset determining the end of element n are in 464+ * order. This guarantees that the bytes of element 465+ * n don't overlap with any previous element. 466+ * 467+ * For trusted data this is set to G_MAXSIZE and we 468+ * don't check that the frame offsets are in order. 469+ * 470+ * Note: This doesn't imply the offsets are good in 471+ * any way apart from their ordering. In particular 472+ * offsets may be out of bounds for this value or 473+ * may imply that the data overlaps the frame 474+ * offsets themselves. 475+ * 476+ * This field is only relevant for arrays of non 477+ * fixed width types. 478+ * 479 * .tree: Only valid when the instance is in tree form. 480 * 481 * Note that accesses from other threads could result in 482@@ -367,6 +386,7 @@ g_variant_to_serialised (GVariant *value) 483 (gpointer) value->contents.serialised.data, 484 value->size, 485 value->depth, 486+ value->contents.serialised.ordered_offsets_up_to, 487 }; 488 return serialised; 489 } 490@@ -398,6 +418,7 @@ g_variant_serialise (GVariant *value, 491 serialised.size = value->size; 492 serialised.data = data; 493 serialised.depth = value->depth; 494+ serialised.ordered_offsets_up_to = 0; 495 496 children = (gpointer *) value->contents.tree.children; 497 n_children = value->contents.tree.n_children; 498@@ -441,6 +462,15 @@ g_variant_fill_gvs (GVariantSerialised *serialised, 499 g_assert (serialised->size == value->size); 500 serialised->depth = value->depth; 501 502+ if (value->state & STATE_SERIALISED) 503+ { 504+ serialised->ordered_offsets_up_to = value->contents.serialised.ordered_offsets_up_to; 505+ } 506+ else 507+ { 508+ serialised->ordered_offsets_up_to = 0; 509+ } 510+ 511 if (serialised->data) 512 /* g_variant_store() is a public API, so it 513 * it will reacquire the lock if it needs to. 514@@ -483,6 +513,7 @@ g_variant_ensure_serialised (GVariant *value) 515 bytes = g_bytes_new_take (data, value->size); 516 value->contents.serialised.data = g_bytes_get_data (bytes, NULL); 517 value->contents.serialised.bytes = bytes; 518+ value->contents.serialised.ordered_offsets_up_to = G_MAXSIZE; 519 value->state |= STATE_SERIALISED; 520 } 521 } 522@@ -563,6 +594,7 @@ g_variant_new_from_bytes (const GVariantType *type, 523 serialised.type_info = value->type_info; 524 serialised.data = (guchar *) g_bytes_get_data (bytes, &serialised.size); 525 serialised.depth = 0; 526+ serialised.ordered_offsets_up_to = trusted ? G_MAXSIZE : 0; 527 528 if (!g_variant_serialised_check (serialised)) 529 { 530@@ -613,6 +645,8 @@ g_variant_new_from_bytes (const GVariantType *type, 531 value->contents.serialised.data = g_bytes_get_data (bytes, &value->size); 532 } 533 534+ value->contents.serialised.ordered_offsets_up_to = trusted ? G_MAXSIZE : 0; 535+ 536 g_clear_pointer (&owned_bytes, g_bytes_unref); 537 538 return value; 539@@ -1132,6 +1166,7 @@ g_variant_get_child_value (GVariant *value, 540 child->contents.serialised.bytes = 541 g_bytes_ref (value->contents.serialised.bytes); 542 child->contents.serialised.data = s_child.data; 543+ child->contents.serialised.ordered_offsets_up_to = s_child.ordered_offsets_up_to; 544 545 return child; 546 } 547diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 548index 1eaa80b29f..c33a903bff 100644 549--- a/glib/gvariant-serialiser.c 550+++ b/glib/gvariant-serialiser.c 551@@ -1,6 +1,7 @@ 552 /* 553 * Copyright © 2007, 2008 Ryan Lortie 554 * Copyright © 2010 Codethink Limited 555+ * Copyright © 2020 William Manley 556 * 557 * This library is free software; you can redistribute it and/or 558 * modify it under the terms of the GNU Lesser General Public 559@@ -266,6 +267,7 @@ gvs_fixed_sized_maybe_get_child (GVariantSerialised value, 560 value.type_info = g_variant_type_info_element (value.type_info); 561 g_variant_type_info_ref (value.type_info); 562 value.depth++; 563+ value.ordered_offsets_up_to = 0; 564 565 return value; 566 } 567@@ -297,7 +299,7 @@ gvs_fixed_sized_maybe_serialise (GVariantSerialised value, 568 { 569 if (n_children) 570 { 571- GVariantSerialised child = { NULL, value.data, value.size, value.depth + 1 }; 572+ GVariantSerialised child = { NULL, value.data, value.size, value.depth + 1, 0 }; 573 574 gvs_filler (&child, children[0]); 575 } 576@@ -319,6 +321,7 @@ gvs_fixed_sized_maybe_is_normal (GVariantSerialised value) 577 /* proper element size: "Just". recurse to the child. */ 578 value.type_info = g_variant_type_info_element (value.type_info); 579 value.depth++; 580+ value.ordered_offsets_up_to = 0; 581 582 return g_variant_serialised_is_normal (value); 583 } 584@@ -360,6 +363,7 @@ gvs_variable_sized_maybe_get_child (GVariantSerialised value, 585 value.data = NULL; 586 587 value.depth++; 588+ value.ordered_offsets_up_to = 0; 589 590 return value; 591 } 592@@ -390,7 +394,7 @@ gvs_variable_sized_maybe_serialise (GVariantSerialised value, 593 { 594 if (n_children) 595 { 596- GVariantSerialised child = { NULL, value.data, value.size - 1, value.depth + 1 }; 597+ GVariantSerialised child = { NULL, value.data, value.size - 1, value.depth + 1, 0 }; 598 599 /* write the data for the child. */ 600 gvs_filler (&child, children[0]); 601@@ -410,6 +414,7 @@ gvs_variable_sized_maybe_is_normal (GVariantSerialised value) 602 value.type_info = g_variant_type_info_element (value.type_info); 603 value.size--; 604 value.depth++; 605+ value.ordered_offsets_up_to = 0; 606 607 return g_variant_serialised_is_normal (value); 608 } 609@@ -693,6 +698,32 @@ gvs_variable_sized_array_n_children (GVariantSerialised value) 610 return gvs_variable_sized_array_get_frame_offsets (value).length; 611 } 612 613+/* Find the index of the first out-of-order element in @data, assuming that 614+ * @data is an array of elements of given @type, starting at index @start and 615+ * containing a further @len-@start elements. */ 616+#define DEFINE_FIND_UNORDERED(type) \ 617+ static gsize \ 618+ find_unordered_##type (const guint8 *data, gsize start, gsize len) \ 619+ { \ 620+ gsize off; \ 621+ type current, previous; \ 622+ \ 623+ memcpy (&previous, data + start * sizeof (current), sizeof (current)); \ 624+ for (off = (start + 1) * sizeof (current); off < len * sizeof (current); off += sizeof (current)) \ 625+ { \ 626+ memcpy (¤t, data + off, sizeof (current)); \ 627+ if (current < previous) \ 628+ break; \ 629+ previous = current; \ 630+ } \ 631+ return off / sizeof (current) - 1; \ 632+ } 633+ 634+DEFINE_FIND_UNORDERED (guint8); 635+DEFINE_FIND_UNORDERED (guint16); 636+DEFINE_FIND_UNORDERED (guint32); 637+DEFINE_FIND_UNORDERED (guint64); 638+ 639 static GVariantSerialised 640 gvs_variable_sized_array_get_child (GVariantSerialised value, 641 gsize index_) 642@@ -708,6 +739,49 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, 643 g_variant_type_info_ref (child.type_info); 644 child.depth = value.depth + 1; 645 646+ /* If the requested @index_ is beyond the set of indices whose framing offsets 647+ * have been checked, check the remaining offsets to see whether they’re 648+ * normal (in order, no overlapping array elements). */ 649+ if (index_ > value.ordered_offsets_up_to) 650+ { 651+ switch (offsets.offset_size) 652+ { 653+ case 1: 654+ { 655+ value.ordered_offsets_up_to = find_unordered_guint8 ( 656+ offsets.array, value.ordered_offsets_up_to, index_ + 1); 657+ break; 658+ } 659+ case 2: 660+ { 661+ value.ordered_offsets_up_to = find_unordered_guint16 ( 662+ offsets.array, value.ordered_offsets_up_to, index_ + 1); 663+ break; 664+ } 665+ case 4: 666+ { 667+ value.ordered_offsets_up_to = find_unordered_guint32 ( 668+ offsets.array, value.ordered_offsets_up_to, index_ + 1); 669+ break; 670+ } 671+ case 8: 672+ { 673+ value.ordered_offsets_up_to = find_unordered_guint64 ( 674+ offsets.array, value.ordered_offsets_up_to, index_ + 1); 675+ break; 676+ } 677+ default: 678+ /* gvs_get_offset_size() only returns maximum 8 */ 679+ g_assert_not_reached (); 680+ } 681+ } 682+ 683+ if (index_ > value.ordered_offsets_up_to) 684+ { 685+ /* Offsets are invalid somewhere, so return an empty child. */ 686+ return child; 687+ } 688+ 689 if (index_ > 0) 690 { 691 guint alignment; 692@@ -842,6 +916,9 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 693 694 g_assert (offset == offsets.data_size); 695 696+ /* All offsets have now been checked. */ 697+ value.ordered_offsets_up_to = G_MAXSIZE; 698+ 699 return TRUE; 700 } 701 702@@ -1078,7 +1155,7 @@ gvs_tuple_is_normal (GVariantSerialised value) 703 for (i = 0; i < length; i++) 704 { 705 const GVariantMemberInfo *member_info; 706- GVariantSerialised child; 707+ GVariantSerialised child = { 0, }; 708 gsize fixed_size; 709 guint alignment; 710 gsize end; 711@@ -1138,6 +1215,9 @@ gvs_tuple_is_normal (GVariantSerialised value) 712 offset = end; 713 } 714 715+ /* All element bounds have been checked above. */ 716+ value.ordered_offsets_up_to = G_MAXSIZE; 717+ 718 { 719 gsize fixed_size; 720 guint alignment; 721diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h 722index 6ced7e3d6c..a1bccb834b 100644 723--- a/glib/gvariant-serialiser.h 724+++ b/glib/gvariant-serialiser.h 725@@ -31,6 +31,15 @@ typedef struct 726 guchar *data; 727 gsize size; 728 gsize depth; /* same semantics as GVariant.depth */ 729+ 730+ /* If ordered_offsets_up_to == n this means that all the frame offsets up to and 731+ * including the frame offset determining the end of element n are in order. 732+ * This guarantees that the bytes of element n don't overlap with any previous 733+ * element. 734+ * 735+ * This is both read and set by g_variant_serialised_get_child for arrays of 736+ * non-fixed-width types */ 737+ gsize ordered_offsets_up_to; 738 } GVariantSerialised; 739 740 /* deserialization */ 741diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 742index 18fa855aeb..45e302c926 100644 743--- a/glib/tests/gvariant.c 744+++ b/glib/tests/gvariant.c 745@@ -1,5 +1,6 @@ 746 /* 747 * Copyright © 2010 Codethink Limited 748+ * Copyright © 2020 William Manley 749 * 750 * This library is free software; you can redistribute it and/or 751 * modify it under the terms of the GNU Lesser General Public 752@@ -1281,6 +1282,7 @@ random_instance_filler (GVariantSerialised *serialised, 753 serialised->size = instance->size; 754 755 serialised->depth = 0; 756+ serialised->ordered_offsets_up_to = 0; 757 758 g_assert_true (serialised->type_info == instance->type_info); 759 g_assert_cmpuint (serialised->size, ==, instance->size); 760@@ -5143,6 +5145,47 @@ test_normal_checking_array_offsets (void) 761 g_variant_unref (variant); 762 } 763 764+/* This is a regression test that we can't have non-normal values that take up 765+ * significantly more space than the normal equivalent, by specifying the 766+ * offset table entries so that array elements overlap. 767+ * 768+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_832242 */ 769+static void 770+test_normal_checking_array_offsets2 (void) 771+{ 772+ const guint8 data[] = { 773+ 'h', 'i', '\0', 774+ 0x03, 0x00, 0x03, 775+ 0x06, 0x00, 0x06, 776+ 0x09, 0x00, 0x09, 777+ 0x0c, 0x00, 0x0c, 778+ 0x0f, 0x00, 0x0f, 779+ 0x12, 0x00, 0x12, 780+ 0x15, 0x00, 0x15, 781+ }; 782+ gsize size = sizeof (data); 783+ const GVariantType *aaaaaaas = G_VARIANT_TYPE ("aaaaaaas"); 784+ GVariant *variant = NULL; 785+ GVariant *normal_variant = NULL; 786+ GVariant *expected = NULL; 787+ 788+ variant = g_variant_new_from_data (aaaaaaas, data, size, FALSE, NULL, NULL); 789+ g_assert_nonnull (variant); 790+ 791+ normal_variant = g_variant_get_normal_form (variant); 792+ g_assert_nonnull (normal_variant); 793+ g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 2); 794+ 795+ expected = g_variant_new_parsed ( 796+ "[[[[[[['hi', '', ''], [], []], [], []], [], []], [], []], [], []], [], []]"); 797+ g_assert_cmpvariant (expected, variant); 798+ g_assert_cmpvariant (expected, normal_variant); 799+ 800+ g_variant_unref (expected); 801+ g_variant_unref (normal_variant); 802+ g_variant_unref (variant); 803+} 804+ 805 /* Test that a tuple with invalidly large values in its offset table is 806 * normalised successfully without looping infinitely. */ 807 static void 808@@ -5311,6 +5354,8 @@ main (int argc, char **argv) 809 test_normal_checking_tuples); 810 g_test_add_func ("/gvariant/normal-checking/array-offsets", 811 test_normal_checking_array_offsets); 812+ g_test_add_func ("/gvariant/normal-checking/array-offsets2", 813+ test_normal_checking_array_offsets2); 814 g_test_add_func ("/gvariant/normal-checking/tuple-offsets", 815 test_normal_checking_tuple_offsets); 816 g_test_add_func ("/gvariant/normal-checking/empty-object-path", 817-- 818GitLab 819 820 821From 345cae9c1aa7bf6752039225ef4c8d8d69fa8d76 Mon Sep 17 00:00:00 2001 822From: Philip Withnall <pwithnall@endlessos.org> 823Date: Fri, 7 Jan 2022 15:03:52 +0000 824Subject: [PATCH 05/18] gvariant-serialiser: Factor out code to get bounds of a 825 tuple member 826 827This introduces no functional changes. 828 829Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 830 831Helps: #2121 832--- 833 glib/gvariant-serialiser.c | 73 ++++++++++++++++++++++++-------------- 834 1 file changed, 46 insertions(+), 27 deletions(-) 835 836diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 837index c33a903bff..5b67b3820f 100644 838--- a/glib/gvariant-serialiser.c 839+++ b/glib/gvariant-serialiser.c 840@@ -944,6 +944,51 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 841 * for the tuple. See the notes in gvarianttypeinfo.h. 842 */ 843 844+static void 845+gvs_tuple_get_member_bounds (GVariantSerialised value, 846+ gsize index_, 847+ gsize offset_size, 848+ gsize *out_member_start, 849+ gsize *out_member_end) 850+{ 851+ const GVariantMemberInfo *member_info; 852+ gsize member_start, member_end; 853+ 854+ member_info = g_variant_type_info_member_info (value.type_info, index_); 855+ 856+ if (member_info->i + 1) 857+ member_start = gvs_read_unaligned_le (value.data + value.size - 858+ offset_size * (member_info->i + 1), 859+ offset_size); 860+ else 861+ member_start = 0; 862+ 863+ member_start += member_info->a; 864+ member_start &= member_info->b; 865+ member_start |= member_info->c; 866+ 867+ if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST) 868+ member_end = value.size - offset_size * (member_info->i + 1); 869+ 870+ else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_FIXED) 871+ { 872+ gsize fixed_size; 873+ 874+ g_variant_type_info_query (member_info->type_info, NULL, &fixed_size); 875+ member_end = member_start + fixed_size; 876+ } 877+ 878+ else /* G_VARIANT_MEMBER_ENDING_OFFSET */ 879+ member_end = gvs_read_unaligned_le (value.data + value.size - 880+ offset_size * (member_info->i + 2), 881+ offset_size); 882+ 883+ if (out_member_start != NULL) 884+ *out_member_start = member_start; 885+ if (out_member_end != NULL) 886+ *out_member_end = member_end; 887+} 888+ 889 static gsize 890 gvs_tuple_n_children (GVariantSerialised value) 891 { 892@@ -999,33 +1044,7 @@ gvs_tuple_get_child (GVariantSerialised value, 893 } 894 } 895 896- if (member_info->i + 1) 897- start = gvs_read_unaligned_le (value.data + value.size - 898- offset_size * (member_info->i + 1), 899- offset_size); 900- else 901- start = 0; 902- 903- start += member_info->a; 904- start &= member_info->b; 905- start |= member_info->c; 906- 907- if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST) 908- end = value.size - offset_size * (member_info->i + 1); 909- 910- else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_FIXED) 911- { 912- gsize fixed_size; 913- 914- g_variant_type_info_query (child.type_info, NULL, &fixed_size); 915- end = start + fixed_size; 916- child.size = fixed_size; 917- } 918- 919- else /* G_VARIANT_MEMBER_ENDING_OFFSET */ 920- end = gvs_read_unaligned_le (value.data + value.size - 921- offset_size * (member_info->i + 2), 922- offset_size); 923+ gvs_tuple_get_member_bounds (value, index_, offset_size, &start, &end); 924 925 /* The child should not extend into the offset table. */ 926 if (index_ != g_variant_type_info_n_members (value.type_info) - 1) 927-- 928GitLab 929 930 931From 73d0aa81c2575a5c9ae77dcb94da919579014fc0 Mon Sep 17 00:00:00 2001 932From: Philip Withnall <pwithnall@endlessos.org> 933Date: Fri, 7 Jan 2022 16:37:29 +0000 934Subject: [PATCH 06/18] gvariant-serialiser: Rework child size calculation 935MIME-Version: 1.0 936Content-Type: text/plain; charset=UTF-8 937Content-Transfer-Encoding: 8bit 938 939This reduces a few duplicate calls to `g_variant_type_info_query()` and 940explains why they’re needed. 941 942Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 943 944Helps: #2121 945--- 946 glib/gvariant-serialiser.c | 31 +++++++++---------------------- 947 1 file changed, 9 insertions(+), 22 deletions(-) 948 949diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 950index 5b67b3820f..bc131b7514 100644 951--- a/glib/gvariant-serialiser.c 952+++ b/glib/gvariant-serialiser.c 953@@ -1009,14 +1009,18 @@ gvs_tuple_get_child (GVariantSerialised value, 954 child.depth = value.depth + 1; 955 offset_size = gvs_get_offset_size (value.size); 956 957+ /* Ensure the size is set for fixed-sized children, or 958+ * g_variant_serialised_check() will fail, even if we return 959+ * (child.data == NULL) to indicate an error. */ 960+ if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_FIXED) 961+ g_variant_type_info_query (child.type_info, NULL, &child.size); 962+ 963 /* tuples are the only (potentially) fixed-sized containers, so the 964 * only ones that have to deal with the possibility of having %NULL 965 * data with a non-zero %size if errors occurred elsewhere. 966 */ 967 if G_UNLIKELY (value.data == NULL && value.size != 0) 968 { 969- g_variant_type_info_query (child.type_info, NULL, &child.size); 970- 971 /* this can only happen in fixed-sized tuples, 972 * so the child must also be fixed sized. 973 */ 974@@ -1034,29 +1038,12 @@ gvs_tuple_get_child (GVariantSerialised value, 975 else 976 { 977 if (offset_size * (member_info->i + 1) > value.size) 978- { 979- /* if the child is fixed size, return its size. 980- * if child is not fixed-sized, return size = 0. 981- */ 982- g_variant_type_info_query (child.type_info, NULL, &child.size); 983- 984- return child; 985- } 986+ return child; 987 } 988 989- gvs_tuple_get_member_bounds (value, index_, offset_size, &start, &end); 990- 991 /* The child should not extend into the offset table. */ 992- if (index_ != g_variant_type_info_n_members (value.type_info) - 1) 993- { 994- GVariantSerialised last_child; 995- last_child = gvs_tuple_get_child (value, 996- g_variant_type_info_n_members (value.type_info) - 1); 997- last_end = last_child.data + last_child.size - value.data; 998- g_variant_type_info_unref (last_child.type_info); 999- } 1000- else 1001- last_end = end; 1002+ gvs_tuple_get_member_bounds (value, index_, offset_size, &start, &end); 1003+ gvs_tuple_get_member_bounds (value, g_variant_type_info_n_members (value.type_info) - 1, offset_size, NULL, &last_end); 1004 1005 if (start < end && end <= value.size && end <= last_end) 1006 { 1007-- 1008GitLab 1009 1010 1011From 7cf6f5b69146d20948d42f0c476688fe17fef787 Mon Sep 17 00:00:00 2001 1012From: Philip Withnall <pwithnall@endlessos.org> 1013Date: Fri, 7 Jan 2022 16:42:14 +0000 1014Subject: [PATCH 07/18] =?UTF-8?q?gvariant:=20Don=E2=80=99t=20allow=20child?= 1015 =?UTF-8?q?=20elements=20of=20a=20tuple=20to=20overlap=20each=20other?= 1016MIME-Version: 1.0 1017Content-Type: text/plain; charset=UTF-8 1018Content-Transfer-Encoding: 8bit 1019 1020This is similar to the earlier commit which prevents child elements of a 1021variable-sized array from overlapping each other, but this time for 1022tuples. It is based heavily on ideas by William Manley. 1023 1024Tuples are slightly different from variable-sized arrays in that they 1025contain a mixture of fixed and variable sized elements. All but one of 1026the variable sized elements have an entry in the frame offsets table. 1027This means that if we were to just check the ordering of the frame 1028offsets table, the variable sized elements could still overlap 1029interleaving fixed sized elements, which would be bad. 1030 1031Therefore we have to check the elements rather than the frame offsets. 1032 1033The logic of checking the elements up to the index currently being 1034requested, and caching the result in `ordered_offsets_up_to`, means that 1035the algorithmic cost implications are the same for this commit as for 1036variable-sized arrays: an O(N) cost for these checks is amortised out 1037over N accesses to O(1) per access. 1038 1039Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 1040 1041Fixes: #2121 1042--- 1043 glib/gvariant-core.c | 6 +- 1044 glib/gvariant-serialiser.c | 40 ++++++++ 1045 glib/gvariant-serialiser.h | 7 +- 1046 glib/gvariant.c | 1 + 1047 glib/tests/gvariant.c | 181 +++++++++++++++++++++++++++++++++++++ 1048 5 files changed, 232 insertions(+), 3 deletions(-) 1049 1050diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 1051index f25d472794..4419371136 100644 1052--- a/glib/gvariant-core.c 1053+++ b/glib/gvariant-core.c 1054@@ -1,6 +1,7 @@ 1055 /* 1056 * Copyright © 2007, 2008 Ryan Lortie 1057 * Copyright © 2010 Codethink Limited 1058+ * Copyright © 2022 Endless OS Foundation, LLC 1059 * 1060 * This library is free software; you can redistribute it and/or 1061 * modify it under the terms of the GNU Lesser General Public 1062@@ -181,7 +182,7 @@ struct _GVariant 1063 * offsets themselves. 1064 * 1065 * This field is only relevant for arrays of non 1066- * fixed width types. 1067+ * fixed width types and for tuples. 1068 * 1069 * .tree: Only valid when the instance is in tree form. 1070 * 1071@@ -1141,6 +1142,9 @@ g_variant_get_child_value (GVariant *value, 1072 */ 1073 s_child = g_variant_serialised_get_child (serialised, index_); 1074 1075+ /* Update the cached ordered_offsets_up_to, since @serialised will be thrown away when this function exits */ 1076+ value->contents.serialised.ordered_offsets_up_to = MAX (value->contents.serialised.ordered_offsets_up_to, serialised.ordered_offsets_up_to); 1077+ 1078 /* Check whether this would cause nesting too deep. If so, return a fake 1079 * child. The only situation we expect this to happen in is with a variant, 1080 * as all other deeply-nested types have a static type, and hence should 1081diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 1082index bc131b7514..263b74048c 100644 1083--- a/glib/gvariant-serialiser.c 1084+++ b/glib/gvariant-serialiser.c 1085@@ -944,6 +944,10 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 1086 * for the tuple. See the notes in gvarianttypeinfo.h. 1087 */ 1088 1089+/* Note: This doesn’t guarantee that @out_member_end >= @out_member_start; that 1090+ * condition may not hold true for invalid serialised variants. The caller is 1091+ * responsible for checking the returned values and handling invalid ones 1092+ * appropriately. */ 1093 static void 1094 gvs_tuple_get_member_bounds (GVariantSerialised value, 1095 gsize index_, 1096@@ -1030,6 +1034,42 @@ gvs_tuple_get_child (GVariantSerialised value, 1097 return child; 1098 } 1099 1100+ /* If the requested @index_ is beyond the set of indices whose framing offsets 1101+ * have been checked, check the remaining offsets to see whether they’re 1102+ * normal (in order, no overlapping tuple elements). 1103+ * 1104+ * Unlike the checks in gvs_variable_sized_array_get_child(), we have to check 1105+ * all the tuple *elements* here, not just all the framing offsets, since 1106+ * tuples contain a mix of elements which use framing offsets and ones which 1107+ * don’t. None of them are allowed to overlap. */ 1108+ if (index_ > value.ordered_offsets_up_to) 1109+ { 1110+ gsize i, prev_i_end = 0; 1111+ 1112+ if (value.ordered_offsets_up_to > 0) 1113+ gvs_tuple_get_member_bounds (value, value.ordered_offsets_up_to - 1, offset_size, NULL, &prev_i_end); 1114+ 1115+ for (i = value.ordered_offsets_up_to; i <= index_; i++) 1116+ { 1117+ gsize i_start, i_end; 1118+ 1119+ gvs_tuple_get_member_bounds (value, i, offset_size, &i_start, &i_end); 1120+ 1121+ if (i_start > i_end || i_start < prev_i_end || i_end > value.size) 1122+ break; 1123+ 1124+ prev_i_end = i_end; 1125+ } 1126+ 1127+ value.ordered_offsets_up_to = i - 1; 1128+ } 1129+ 1130+ if (index_ > value.ordered_offsets_up_to) 1131+ { 1132+ /* Offsets are invalid somewhere, so return an empty child. */ 1133+ return child; 1134+ } 1135+ 1136 if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET) 1137 { 1138 if (offset_size * (member_info->i + 2) > value.size) 1139diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h 1140index a1bccb834b..661bd8fd1b 100644 1141--- a/glib/gvariant-serialiser.h 1142+++ b/glib/gvariant-serialiser.h 1143@@ -37,8 +37,11 @@ typedef struct 1144 * This guarantees that the bytes of element n don't overlap with any previous 1145 * element. 1146 * 1147- * This is both read and set by g_variant_serialised_get_child for arrays of 1148- * non-fixed-width types */ 1149+ * This is both read and set by g_variant_serialised_get_child() for arrays of 1150+ * non-fixed-width types, and for tuples. 1151+ * 1152+ * Even when dealing with tuples, @ordered_offsets_up_to is an element index, 1153+ * rather than an index into the frame offsets. */ 1154 gsize ordered_offsets_up_to; 1155 } GVariantSerialised; 1156 1157diff --git a/glib/gvariant.c b/glib/gvariant.c 1158index e4c85ca636..4f0d6b83c6 100644 1159--- a/glib/gvariant.c 1160+++ b/glib/gvariant.c 1161@@ -6011,6 +6011,7 @@ g_variant_byteswap (GVariant *value) 1162 serialised.size = g_variant_get_size (trusted); 1163 serialised.data = g_malloc (serialised.size); 1164 serialised.depth = g_variant_get_depth (trusted); 1165+ serialised.ordered_offsets_up_to = G_MAXSIZE; /* operating on the normal form */ 1166 g_variant_store (trusted, serialised.data); 1167 g_variant_unref (trusted); 1168 1169diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 1170index 45e302c926..4ae52e2bb9 100644 1171--- a/glib/tests/gvariant.c 1172+++ b/glib/tests/gvariant.c 1173@@ -1,6 +1,7 @@ 1174 /* 1175 * Copyright © 2010 Codethink Limited 1176 * Copyright © 2020 William Manley 1177+ * Copyright © 2022 Endless OS Foundation, LLC 1178 * 1179 * This library is free software; you can redistribute it and/or 1180 * modify it under the terms of the GNU Lesser General Public 1181@@ -1449,6 +1450,7 @@ test_maybe (void) 1182 serialised.data = flavoured_malloc (needed_size, flavour); 1183 serialised.size = needed_size; 1184 serialised.depth = 0; 1185+ serialised.ordered_offsets_up_to = 0; 1186 1187 g_variant_serialiser_serialise (serialised, 1188 random_instance_filler, 1189@@ -1572,6 +1574,7 @@ test_array (void) 1190 serialised.data = flavoured_malloc (needed_size, flavour); 1191 serialised.size = needed_size; 1192 serialised.depth = 0; 1193+ serialised.ordered_offsets_up_to = 0; 1194 1195 g_variant_serialiser_serialise (serialised, random_instance_filler, 1196 (gpointer *) instances, n_children); 1197@@ -1736,6 +1739,7 @@ test_tuple (void) 1198 serialised.data = flavoured_malloc (needed_size, flavour); 1199 serialised.size = needed_size; 1200 serialised.depth = 0; 1201+ serialised.ordered_offsets_up_to = 0; 1202 1203 g_variant_serialiser_serialise (serialised, random_instance_filler, 1204 (gpointer *) instances, n_children); 1205@@ -1832,6 +1836,7 @@ test_variant (void) 1206 serialised.data = flavoured_malloc (needed_size, flavour); 1207 serialised.size = needed_size; 1208 serialised.depth = 0; 1209+ serialised.ordered_offsets_up_to = 0; 1210 1211 g_variant_serialiser_serialise (serialised, random_instance_filler, 1212 (gpointer *) &instance, 1); 1213@@ -5210,6 +5215,176 @@ test_normal_checking_tuple_offsets (void) 1214 g_variant_unref (variant); 1215 } 1216 1217+/* This is a regression test that we can't have non-normal values that take up 1218+ * significantly more space than the normal equivalent, by specifying the 1219+ * offset table entries so that tuple elements overlap. 1220+ * 1221+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838503 and 1222+ * https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838513 */ 1223+static void 1224+test_normal_checking_tuple_offsets2 (void) 1225+{ 1226+ const GVariantType *data_type = G_VARIANT_TYPE ("(yyaiyyaiyy)"); 1227+ const guint8 data[] = { 1228+ 0x12, 0x34, 0x56, 0x78, 0x01, 1229+ /* 1230+ ^───────────────────┘ 1231+ 1232+ ^^^^^^^^^^ 1st yy 1233+ ^^^^^^^^^^ 2nd yy 1234+ ^^^^^^^^^^ 3rd yy 1235+ ^^^^ Framing offsets 1236+ */ 1237+ 1238+ /* If this variant was encoded normally, it would be something like this: 1239+ * 0x12, 0x34, pad, pad, [array bytes], 0x56, 0x78, pad, pad, [array bytes], 0x9A, 0xBC, 0xXX 1240+ * ^─────────────────────────────────────────────────────┘ 1241+ * 1242+ * ^^^^^^^^^^ 1st yy 1243+ * ^^^^^^^^^^ 2nd yy 1244+ * ^^^^^^^^^^ 3rd yy 1245+ * ^^^^ Framing offsets 1246+ */ 1247+ }; 1248+ gsize size = sizeof (data); 1249+ GVariant *variant = NULL; 1250+ GVariant *normal_variant = NULL; 1251+ GVariant *expected = NULL; 1252+ 1253+ variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); 1254+ g_assert_nonnull (variant); 1255+ 1256+ normal_variant = g_variant_get_normal_form (variant); 1257+ g_assert_nonnull (normal_variant); 1258+ g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); 1259+ 1260+ expected = g_variant_new_parsed ( 1261+ "@(yyaiyyaiyy) (0x12, 0x34, [], 0x00, 0x00, [], 0x00, 0x00)"); 1262+ g_assert_cmpvariant (expected, variant); 1263+ g_assert_cmpvariant (expected, normal_variant); 1264+ 1265+ g_variant_unref (expected); 1266+ g_variant_unref (normal_variant); 1267+ g_variant_unref (variant); 1268+} 1269+ 1270+/* This is a regression test that overlapping entries in the offset table are 1271+ * decoded consistently, even though they’re non-normal. 1272+ * 1273+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */ 1274+static void 1275+test_normal_checking_tuple_offsets3 (void) 1276+{ 1277+ /* The expected decoding of this non-normal byte stream is complex. See 1278+ * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant 1279+ * specification. 1280+ * 1281+ * The rule “Child Values Overlapping Framing Offsets” from the specification 1282+ * says that the first `ay` must be decoded as `[0x01]` even though it 1283+ * overlaps the first byte of the offset table. However, since commit 1284+ * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow 1285+ * this as it’s exploitable. So the first `ay` must be given a default value. 1286+ * 1287+ * The second and third `ay`s must be given default values because of rule 1288+ * “End Boundary Precedes Start Boundary”. 1289+ * 1290+ * The `i` must be given a default value because of rule “Start or End 1291+ * Boundary of a Child Falls Outside the Container”. 1292+ */ 1293+ const GVariantType *data_type = G_VARIANT_TYPE ("(ayayiay)"); 1294+ const guint8 data[] = { 1295+ 0x01, 0x00, 0x02, 1296+ /* 1297+ ^──┘ 1298+ 1299+ ^^^^^^^^^^ 1st ay, bytes 0-2 (but given a default value anyway, see above) 1300+ 2nd ay, bytes 2-0 1301+ i, bytes 0-4 1302+ 3rd ay, bytes 4-1 1303+ ^^^^^^^^^^ Framing offsets 1304+ */ 1305+ }; 1306+ gsize size = sizeof (data); 1307+ GVariant *variant = NULL; 1308+ GVariant *normal_variant = NULL; 1309+ GVariant *expected = NULL; 1310+ 1311+ variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); 1312+ g_assert_nonnull (variant); 1313+ 1314+ g_assert_false (g_variant_is_normal_form (variant)); 1315+ 1316+ normal_variant = g_variant_get_normal_form (variant); 1317+ g_assert_nonnull (normal_variant); 1318+ g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); 1319+ 1320+ expected = g_variant_new_parsed ("@(ayayiay) ([], [], 0, [])"); 1321+ g_assert_cmpvariant (expected, variant); 1322+ g_assert_cmpvariant (expected, normal_variant); 1323+ 1324+ g_variant_unref (expected); 1325+ g_variant_unref (normal_variant); 1326+ g_variant_unref (variant); 1327+} 1328+ 1329+/* This is a regression test that overlapping entries in the offset table are 1330+ * decoded consistently, even though they’re non-normal. 1331+ * 1332+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */ 1333+static void 1334+test_normal_checking_tuple_offsets4 (void) 1335+{ 1336+ /* The expected decoding of this non-normal byte stream is complex. See 1337+ * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant 1338+ * specification. 1339+ * 1340+ * The rule “Child Values Overlapping Framing Offsets” from the specification 1341+ * says that the first `ay` must be decoded as `[0x01]` even though it 1342+ * overlaps the first byte of the offset table. However, since commit 1343+ * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow 1344+ * this as it’s exploitable. So the first `ay` must be given a default value. 1345+ * 1346+ * The second `ay` must be given a default value because of rule “End Boundary 1347+ * Precedes Start Boundary”. 1348+ * 1349+ * The third `ay` must be given a default value because its framing offsets 1350+ * overlap that of the first `ay`. 1351+ */ 1352+ const GVariantType *data_type = G_VARIANT_TYPE ("(ayayay)"); 1353+ const guint8 data[] = { 1354+ 0x01, 0x00, 0x02, 1355+ /* 1356+ ^──┘ 1357+ 1358+ ^^^^^^^^^^ 1st ay, bytes 0-2 (but given a default value anyway, see above) 1359+ 2nd ay, bytes 2-0 1360+ 3rd ay, bytes 0-1 1361+ ^^^^^^^^^^ Framing offsets 1362+ */ 1363+ }; 1364+ gsize size = sizeof (data); 1365+ GVariant *variant = NULL; 1366+ GVariant *normal_variant = NULL; 1367+ GVariant *expected = NULL; 1368+ 1369+ variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL); 1370+ g_assert_nonnull (variant); 1371+ 1372+ g_assert_false (g_variant_is_normal_form (variant)); 1373+ 1374+ normal_variant = g_variant_get_normal_form (variant); 1375+ g_assert_nonnull (normal_variant); 1376+ g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3); 1377+ 1378+ expected = g_variant_new_parsed ("@(ayayay) ([], [], [])"); 1379+ g_assert_cmpvariant (expected, variant); 1380+ g_assert_cmpvariant (expected, normal_variant); 1381+ 1382+ g_variant_unref (expected); 1383+ g_variant_unref (normal_variant); 1384+ g_variant_unref (variant); 1385+} 1386+ 1387 /* Test that an empty object path is normalised successfully to the base object 1388 * path, ‘/’. */ 1389 static void 1390@@ -5358,6 +5533,12 @@ main (int argc, char **argv) 1391 test_normal_checking_array_offsets2); 1392 g_test_add_func ("/gvariant/normal-checking/tuple-offsets", 1393 test_normal_checking_tuple_offsets); 1394+ g_test_add_func ("/gvariant/normal-checking/tuple-offsets2", 1395+ test_normal_checking_tuple_offsets2); 1396+ g_test_add_func ("/gvariant/normal-checking/tuple-offsets3", 1397+ test_normal_checking_tuple_offsets3); 1398+ g_test_add_func ("/gvariant/normal-checking/tuple-offsets4", 1399+ test_normal_checking_tuple_offsets4); 1400 g_test_add_func ("/gvariant/normal-checking/empty-object-path", 1401 test_normal_checking_empty_object_path); 1402 1403-- 1404GitLab 1405 1406 1407From d1a293c4e29880b8d17bb826c9a426a440ca4a91 Mon Sep 17 00:00:00 2001 1408From: Philip Withnall <pwithnall@endlessos.org> 1409Date: Tue, 25 Oct 2022 15:14:14 +0100 1410Subject: [PATCH 08/18] gvariant: Track checked and ordered offsets 1411 independently 1412 1413The past few commits introduced the concept of known-good offsets in the 1414offset table (which is used for variable-width arrays and tuples). 1415Good offsets are ones which are non-overlapping with all the previous 1416offsets in the table. 1417 1418If a bad offset is encountered when indexing into the array or tuple, 1419the cached known-good offset index will not be increased. In this way, 1420all child variants at and beyond the first bad offset can be returned as 1421default values rather than dereferencing potentially invalid data. 1422 1423In this case, there was no information about the fact that the indexes 1424between the highest known-good index and the requested one had been 1425checked already. That could lead to a pathological case where an offset 1426table with an invalid first offset is repeatedly checked in full when 1427trying to access higher-indexed children. 1428 1429Avoid that by storing the index of the highest checked offset in the 1430table, as well as the index of the highest good/ordered offset. 1431 1432Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 1433 1434Helps: #2121 1435--- 1436 glib/gvariant-core.c | 28 ++++++++++++++++++++++++ 1437 glib/gvariant-serialiser.c | 44 +++++++++++++++++++++++++++----------- 1438 glib/gvariant-serialiser.h | 9 ++++++++ 1439 glib/gvariant.c | 1 + 1440 glib/tests/gvariant.c | 5 +++++ 1441 5 files changed, 75 insertions(+), 12 deletions(-) 1442 1443diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 1444index 4419371136..f660e2d578 100644 1445--- a/glib/gvariant-core.c 1446+++ b/glib/gvariant-core.c 1447@@ -69,6 +69,7 @@ struct _GVariant 1448 GBytes *bytes; 1449 gconstpointer data; 1450 gsize ordered_offsets_up_to; 1451+ gsize checked_offsets_up_to; 1452 } serialised; 1453 1454 struct 1455@@ -184,6 +185,24 @@ struct _GVariant 1456 * This field is only relevant for arrays of non 1457 * fixed width types and for tuples. 1458 * 1459+ * .checked_offsets_up_to: Similarly to .ordered_offsets_up_to, this stores 1460+ * the index of the highest element, n, whose frame 1461+ * offsets (and all the preceding frame offsets) 1462+ * have been checked for validity. 1463+ * 1464+ * It is always the case that 1465+ * .checked_offsets_up_to ≥ .ordered_offsets_up_to. 1466+ * 1467+ * If .checked_offsets_up_to == .ordered_offsets_up_to, 1468+ * then a bad offset has not been found so far. 1469+ * 1470+ * If .checked_offsets_up_to > .ordered_offsets_up_to, 1471+ * then a bad offset has been found at 1472+ * (.ordered_offsets_up_to + 1). 1473+ * 1474+ * This field is only relevant for arrays of non 1475+ * fixed width types and for tuples. 1476+ * 1477 * .tree: Only valid when the instance is in tree form. 1478 * 1479 * Note that accesses from other threads could result in 1480@@ -388,6 +407,7 @@ g_variant_to_serialised (GVariant *value) 1481 value->size, 1482 value->depth, 1483 value->contents.serialised.ordered_offsets_up_to, 1484+ value->contents.serialised.checked_offsets_up_to, 1485 }; 1486 return serialised; 1487 } 1488@@ -420,6 +440,7 @@ g_variant_serialise (GVariant *value, 1489 serialised.data = data; 1490 serialised.depth = value->depth; 1491 serialised.ordered_offsets_up_to = 0; 1492+ serialised.checked_offsets_up_to = 0; 1493 1494 children = (gpointer *) value->contents.tree.children; 1495 n_children = value->contents.tree.n_children; 1496@@ -466,10 +487,12 @@ g_variant_fill_gvs (GVariantSerialised *serialised, 1497 if (value->state & STATE_SERIALISED) 1498 { 1499 serialised->ordered_offsets_up_to = value->contents.serialised.ordered_offsets_up_to; 1500+ serialised->checked_offsets_up_to = value->contents.serialised.checked_offsets_up_to; 1501 } 1502 else 1503 { 1504 serialised->ordered_offsets_up_to = 0; 1505+ serialised->checked_offsets_up_to = 0; 1506 } 1507 1508 if (serialised->data) 1509@@ -515,6 +538,7 @@ g_variant_ensure_serialised (GVariant *value) 1510 value->contents.serialised.data = g_bytes_get_data (bytes, NULL); 1511 value->contents.serialised.bytes = bytes; 1512 value->contents.serialised.ordered_offsets_up_to = G_MAXSIZE; 1513+ value->contents.serialised.checked_offsets_up_to = G_MAXSIZE; 1514 value->state |= STATE_SERIALISED; 1515 } 1516 } 1517@@ -596,6 +620,7 @@ g_variant_new_from_bytes (const GVariantType *type, 1518 serialised.data = (guchar *) g_bytes_get_data (bytes, &serialised.size); 1519 serialised.depth = 0; 1520 serialised.ordered_offsets_up_to = trusted ? G_MAXSIZE : 0; 1521+ serialised.checked_offsets_up_to = trusted ? G_MAXSIZE : 0; 1522 1523 if (!g_variant_serialised_check (serialised)) 1524 { 1525@@ -647,6 +672,7 @@ g_variant_new_from_bytes (const GVariantType *type, 1526 } 1527 1528 value->contents.serialised.ordered_offsets_up_to = trusted ? G_MAXSIZE : 0; 1529+ value->contents.serialised.checked_offsets_up_to = trusted ? G_MAXSIZE : 0; 1530 1531 g_clear_pointer (&owned_bytes, g_bytes_unref); 1532 1533@@ -1144,6 +1170,7 @@ g_variant_get_child_value (GVariant *value, 1534 1535 /* Update the cached ordered_offsets_up_to, since @serialised will be thrown away when this function exits */ 1536 value->contents.serialised.ordered_offsets_up_to = MAX (value->contents.serialised.ordered_offsets_up_to, serialised.ordered_offsets_up_to); 1537+ value->contents.serialised.checked_offsets_up_to = MAX (value->contents.serialised.checked_offsets_up_to, serialised.checked_offsets_up_to); 1538 1539 /* Check whether this would cause nesting too deep. If so, return a fake 1540 * child. The only situation we expect this to happen in is with a variant, 1541@@ -1171,6 +1198,7 @@ g_variant_get_child_value (GVariant *value, 1542 g_bytes_ref (value->contents.serialised.bytes); 1543 child->contents.serialised.data = s_child.data; 1544 child->contents.serialised.ordered_offsets_up_to = s_child.ordered_offsets_up_to; 1545+ child->contents.serialised.checked_offsets_up_to = s_child.checked_offsets_up_to; 1546 1547 return child; 1548 } 1549diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 1550index 263b74048c..69baeeb395 100644 1551--- a/glib/gvariant-serialiser.c 1552+++ b/glib/gvariant-serialiser.c 1553@@ -122,6 +122,8 @@ 1554 * 1555 * @depth has no restrictions; the depth of a top-level serialized #GVariant is 1556 * zero, and it increases for each level of nested child. 1557+ * 1558+ * @checked_offsets_up_to is always ≥ @ordered_offsets_up_to 1559 */ 1560 1561 /* < private > 1562@@ -149,6 +151,9 @@ g_variant_serialised_check (GVariantSerialised serialised) 1563 !(serialised.size == 0 || serialised.data != NULL)) 1564 return FALSE; 1565 1566+ if (serialised.ordered_offsets_up_to > serialised.checked_offsets_up_to) 1567+ return FALSE; 1568+ 1569 /* Depending on the native alignment requirements of the machine, the 1570 * compiler will insert either 3 or 7 padding bytes after the char. 1571 * This will result in the sizeof() the struct being 12 or 16. 1572@@ -268,6 +273,7 @@ gvs_fixed_sized_maybe_get_child (GVariantSerialised value, 1573 g_variant_type_info_ref (value.type_info); 1574 value.depth++; 1575 value.ordered_offsets_up_to = 0; 1576+ value.checked_offsets_up_to = 0; 1577 1578 return value; 1579 } 1580@@ -299,7 +305,7 @@ gvs_fixed_sized_maybe_serialise (GVariantSerialised value, 1581 { 1582 if (n_children) 1583 { 1584- GVariantSerialised child = { NULL, value.data, value.size, value.depth + 1, 0 }; 1585+ GVariantSerialised child = { NULL, value.data, value.size, value.depth + 1, 0, 0 }; 1586 1587 gvs_filler (&child, children[0]); 1588 } 1589@@ -322,6 +328,7 @@ gvs_fixed_sized_maybe_is_normal (GVariantSerialised value) 1590 value.type_info = g_variant_type_info_element (value.type_info); 1591 value.depth++; 1592 value.ordered_offsets_up_to = 0; 1593+ value.checked_offsets_up_to = 0; 1594 1595 return g_variant_serialised_is_normal (value); 1596 } 1597@@ -364,6 +371,7 @@ gvs_variable_sized_maybe_get_child (GVariantSerialised value, 1598 1599 value.depth++; 1600 value.ordered_offsets_up_to = 0; 1601+ value.checked_offsets_up_to = 0; 1602 1603 return value; 1604 } 1605@@ -394,7 +402,7 @@ gvs_variable_sized_maybe_serialise (GVariantSerialised value, 1606 { 1607 if (n_children) 1608 { 1609- GVariantSerialised child = { NULL, value.data, value.size - 1, value.depth + 1, 0 }; 1610+ GVariantSerialised child = { NULL, value.data, value.size - 1, value.depth + 1, 0, 0 }; 1611 1612 /* write the data for the child. */ 1613 gvs_filler (&child, children[0]); 1614@@ -415,6 +423,7 @@ gvs_variable_sized_maybe_is_normal (GVariantSerialised value) 1615 value.size--; 1616 value.depth++; 1617 value.ordered_offsets_up_to = 0; 1618+ value.checked_offsets_up_to = 0; 1619 1620 return g_variant_serialised_is_normal (value); 1621 } 1622@@ -741,39 +750,46 @@ gvs_variable_sized_array_get_child (GVariantSerialised value, 1623 1624 /* If the requested @index_ is beyond the set of indices whose framing offsets 1625 * have been checked, check the remaining offsets to see whether they’re 1626- * normal (in order, no overlapping array elements). */ 1627- if (index_ > value.ordered_offsets_up_to) 1628+ * normal (in order, no overlapping array elements). 1629+ * 1630+ * Don’t bother checking if the highest known-good offset is lower than the 1631+ * highest checked offset, as that means there’s an invalid element at that 1632+ * index, so there’s no need to check further. */ 1633+ if (index_ > value.checked_offsets_up_to && 1634+ value.ordered_offsets_up_to == value.checked_offsets_up_to) 1635 { 1636 switch (offsets.offset_size) 1637 { 1638 case 1: 1639 { 1640 value.ordered_offsets_up_to = find_unordered_guint8 ( 1641- offsets.array, value.ordered_offsets_up_to, index_ + 1); 1642+ offsets.array, value.checked_offsets_up_to, index_ + 1); 1643 break; 1644 } 1645 case 2: 1646 { 1647 value.ordered_offsets_up_to = find_unordered_guint16 ( 1648- offsets.array, value.ordered_offsets_up_to, index_ + 1); 1649+ offsets.array, value.checked_offsets_up_to, index_ + 1); 1650 break; 1651 } 1652 case 4: 1653 { 1654 value.ordered_offsets_up_to = find_unordered_guint32 ( 1655- offsets.array, value.ordered_offsets_up_to, index_ + 1); 1656+ offsets.array, value.checked_offsets_up_to, index_ + 1); 1657 break; 1658 } 1659 case 8: 1660 { 1661 value.ordered_offsets_up_to = find_unordered_guint64 ( 1662- offsets.array, value.ordered_offsets_up_to, index_ + 1); 1663+ offsets.array, value.checked_offsets_up_to, index_ + 1); 1664 break; 1665 } 1666 default: 1667 /* gvs_get_offset_size() only returns maximum 8 */ 1668 g_assert_not_reached (); 1669 } 1670+ 1671+ value.checked_offsets_up_to = index_; 1672 } 1673 1674 if (index_ > value.ordered_offsets_up_to) 1675@@ -918,6 +934,7 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value) 1676 1677 /* All offsets have now been checked. */ 1678 value.ordered_offsets_up_to = G_MAXSIZE; 1679+ value.checked_offsets_up_to = G_MAXSIZE; 1680 1681 return TRUE; 1682 } 1683@@ -1042,14 +1059,15 @@ gvs_tuple_get_child (GVariantSerialised value, 1684 * all the tuple *elements* here, not just all the framing offsets, since 1685 * tuples contain a mix of elements which use framing offsets and ones which 1686 * don’t. None of them are allowed to overlap. */ 1687- if (index_ > value.ordered_offsets_up_to) 1688+ if (index_ > value.checked_offsets_up_to && 1689+ value.ordered_offsets_up_to == value.checked_offsets_up_to) 1690 { 1691 gsize i, prev_i_end = 0; 1692 1693- if (value.ordered_offsets_up_to > 0) 1694- gvs_tuple_get_member_bounds (value, value.ordered_offsets_up_to - 1, offset_size, NULL, &prev_i_end); 1695+ if (value.checked_offsets_up_to > 0) 1696+ gvs_tuple_get_member_bounds (value, value.checked_offsets_up_to - 1, offset_size, NULL, &prev_i_end); 1697 1698- for (i = value.ordered_offsets_up_to; i <= index_; i++) 1699+ for (i = value.checked_offsets_up_to; i <= index_; i++) 1700 { 1701 gsize i_start, i_end; 1702 1703@@ -1062,6 +1080,7 @@ gvs_tuple_get_child (GVariantSerialised value, 1704 } 1705 1706 value.ordered_offsets_up_to = i - 1; 1707+ value.checked_offsets_up_to = index_; 1708 } 1709 1710 if (index_ > value.ordered_offsets_up_to) 1711@@ -1263,6 +1282,7 @@ gvs_tuple_is_normal (GVariantSerialised value) 1712 1713 /* All element bounds have been checked above. */ 1714 value.ordered_offsets_up_to = G_MAXSIZE; 1715+ value.checked_offsets_up_to = G_MAXSIZE; 1716 1717 { 1718 gsize fixed_size; 1719diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h 1720index 661bd8fd1b..eb74fe7804 100644 1721--- a/glib/gvariant-serialiser.h 1722+++ b/glib/gvariant-serialiser.h 1723@@ -43,6 +43,15 @@ typedef struct 1724 * Even when dealing with tuples, @ordered_offsets_up_to is an element index, 1725 * rather than an index into the frame offsets. */ 1726 gsize ordered_offsets_up_to; 1727+ 1728+ /* Similar to @ordered_offsets_up_to. This gives the index of the child element 1729+ * whose frame offset is the highest in the offset table which has been 1730+ * checked so far. 1731+ * 1732+ * This is always ≥ @ordered_offsets_up_to. It is always an element index. 1733+ * 1734+ * See documentation in gvariant-core.c for `struct GVariant` for details. */ 1735+ gsize checked_offsets_up_to; 1736 } GVariantSerialised; 1737 1738 /* deserialization */ 1739diff --git a/glib/gvariant.c b/glib/gvariant.c 1740index 4f0d6b83c6..fd066f1732 100644 1741--- a/glib/gvariant.c 1742+++ b/glib/gvariant.c 1743@@ -6012,6 +6012,7 @@ g_variant_byteswap (GVariant *value) 1744 serialised.data = g_malloc (serialised.size); 1745 serialised.depth = g_variant_get_depth (trusted); 1746 serialised.ordered_offsets_up_to = G_MAXSIZE; /* operating on the normal form */ 1747+ serialised.checked_offsets_up_to = G_MAXSIZE; 1748 g_variant_store (trusted, serialised.data); 1749 g_variant_unref (trusted); 1750 1751diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 1752index 4ae52e2bb9..d82aedd509 100644 1753--- a/glib/tests/gvariant.c 1754+++ b/glib/tests/gvariant.c 1755@@ -1284,6 +1284,7 @@ random_instance_filler (GVariantSerialised *serialised, 1756 1757 serialised->depth = 0; 1758 serialised->ordered_offsets_up_to = 0; 1759+ serialised->checked_offsets_up_to = 0; 1760 1761 g_assert_true (serialised->type_info == instance->type_info); 1762 g_assert_cmpuint (serialised->size, ==, instance->size); 1763@@ -1451,6 +1452,7 @@ test_maybe (void) 1764 serialised.size = needed_size; 1765 serialised.depth = 0; 1766 serialised.ordered_offsets_up_to = 0; 1767+ serialised.checked_offsets_up_to = 0; 1768 1769 g_variant_serialiser_serialise (serialised, 1770 random_instance_filler, 1771@@ -1575,6 +1577,7 @@ test_array (void) 1772 serialised.size = needed_size; 1773 serialised.depth = 0; 1774 serialised.ordered_offsets_up_to = 0; 1775+ serialised.checked_offsets_up_to = 0; 1776 1777 g_variant_serialiser_serialise (serialised, random_instance_filler, 1778 (gpointer *) instances, n_children); 1779@@ -1740,6 +1743,7 @@ test_tuple (void) 1780 serialised.size = needed_size; 1781 serialised.depth = 0; 1782 serialised.ordered_offsets_up_to = 0; 1783+ serialised.checked_offsets_up_to = 0; 1784 1785 g_variant_serialiser_serialise (serialised, random_instance_filler, 1786 (gpointer *) instances, n_children); 1787@@ -1837,6 +1841,7 @@ test_variant (void) 1788 serialised.size = needed_size; 1789 serialised.depth = 0; 1790 serialised.ordered_offsets_up_to = 0; 1791+ serialised.checked_offsets_up_to = 0; 1792 1793 g_variant_serialiser_serialise (serialised, random_instance_filler, 1794 (gpointer *) &instance, 1); 1795-- 1796GitLab 1797 1798 1799From 6fa41d5bf61a249a395a83866c10fa4a79bbb7db Mon Sep 17 00:00:00 2001 1800From: Philip Withnall <withnall@endlessm.com> 1801Date: Fri, 12 Jun 2020 18:01:13 +0100 1802Subject: [PATCH 09/18] tests: Add another test for overlapping offsets in 1803 GVariant 1804 1805Signed-off-by: Philip Withnall <withnall@endlessm.com> 1806 1807Helps: #2121 1808--- 1809 glib/tests/gvariant.c | 34 ++++++++++++++++++++++++++++++++++ 1810 1 file changed, 34 insertions(+) 1811 1812diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 1813index d82aedd509..8ec94814e4 100644 1814--- a/glib/tests/gvariant.c 1815+++ b/glib/tests/gvariant.c 1816@@ -5131,6 +5131,38 @@ test_recursion_limits_array_in_variant (void) 1817 g_variant_unref (wrapper_variant); 1818 } 1819 1820+/* Test that a nested array with invalid values in its offset table (which point 1821+ * from the inner to the outer array) is normalised successfully without 1822+ * looping infinitely. */ 1823+static void 1824+test_normal_checking_array_offsets_overlapped (void) 1825+{ 1826+ const guint8 data[] = { 1827+ 0x01, 0x00, 1828+ }; 1829+ gsize size = sizeof (data); 1830+ GVariant *variant = NULL; 1831+ GVariant *normal_variant = NULL; 1832+ GVariant *expected_variant = NULL; 1833+ 1834+ variant = g_variant_new_from_data (G_VARIANT_TYPE ("aay"), data, size, 1835+ FALSE, NULL, NULL); 1836+ g_assert_nonnull (variant); 1837+ 1838+ normal_variant = g_variant_get_normal_form (variant); 1839+ g_assert_nonnull (normal_variant); 1840+ 1841+ expected_variant = g_variant_new_parsed ("[@ay [], []]"); 1842+ g_assert_cmpvariant (normal_variant, expected_variant); 1843+ 1844+ g_assert_cmpmem (g_variant_get_data (normal_variant), g_variant_get_size (normal_variant), 1845+ g_variant_get_data (expected_variant), g_variant_get_size (expected_variant)); 1846+ 1847+ g_variant_unref (expected_variant); 1848+ g_variant_unref (normal_variant); 1849+ g_variant_unref (variant); 1850+} 1851+ 1852 /* Test that an array with invalidly large values in its offset table is 1853 * normalised successfully without looping infinitely. */ 1854 static void 1855@@ -5532,6 +5564,8 @@ main (int argc, char **argv) 1856 1857 g_test_add_func ("/gvariant/normal-checking/tuples", 1858 test_normal_checking_tuples); 1859+ g_test_add_func ("/gvariant/normal-checking/array-offsets/overlapped", 1860+ test_normal_checking_array_offsets_overlapped); 1861 g_test_add_func ("/gvariant/normal-checking/array-offsets", 1862 test_normal_checking_array_offsets); 1863 g_test_add_func ("/gvariant/normal-checking/array-offsets2", 1864-- 1865GitLab 1866 1867 1868From 4c0ddb26bc3f27eddf802b35c4b5229c0cdf6085 Mon Sep 17 00:00:00 2001 1869From: Philip Withnall <pwithnall@endlessos.org> 1870Date: Mon, 24 Oct 2022 16:43:23 +0100 1871Subject: [PATCH 10/18] tests: Disable some random instance tests of GVariants 1872MIME-Version: 1.0 1873Content-Type: text/plain; charset=UTF-8 1874Content-Transfer-Encoding: 8bit 1875 1876Building a `GVariant` using entirely random data may result in a 1877non-normally-formed `GVariant`. It’s always possible to read these 1878`GVariant`s, but the API might return default values for some or all of 1879their components. 1880 1881In particular, this can easily happen when randomly generating the 1882offset tables for non-fixed-width container types. 1883 1884If it does happen, bytewise comparison of the parsed `GVariant` with the 1885original bytes will not always match. So skip those checks. 1886 1887Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 1888 1889Helps: #2121 1890--- 1891 glib/tests/gvariant.c | 12 +++++++++--- 1892 1 file changed, 9 insertions(+), 3 deletions(-) 1893 1894diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 1895index 8ec94814e4..18aeb80f12 100644 1896--- a/glib/tests/gvariant.c 1897+++ b/glib/tests/gvariant.c 1898@@ -1231,6 +1231,7 @@ random_instance_assert (RandomInstance *instance, 1899 GRand *rand; 1900 gsize i; 1901 1902+ g_assert_true (size == 0 || buffer != NULL); 1903 g_assert_cmpint ((gsize) buffer & ALIGN_BITS & instance->alignment, ==, 0); 1904 g_assert_cmpint (size, ==, instance->size); 1905 1906@@ -1457,10 +1458,13 @@ test_maybe (void) 1907 g_variant_serialiser_serialise (serialised, 1908 random_instance_filler, 1909 (gpointer *) &instance, 1); 1910+ 1911 child = g_variant_serialised_get_child (serialised, 0); 1912 g_assert_true (child.type_info == instance->type_info); 1913- random_instance_assert (instance, child.data, child.size); 1914+ if (child.data != NULL) /* could be NULL if element is non-normal */ 1915+ random_instance_assert (instance, child.data, child.size); 1916 g_variant_type_info_unref (child.type_info); 1917+ 1918 flavoured_free (serialised.data, flavour); 1919 } 1920 } 1921@@ -1593,7 +1597,8 @@ test_array (void) 1922 1923 child = g_variant_serialised_get_child (serialised, i); 1924 g_assert_true (child.type_info == instances[i]->type_info); 1925- random_instance_assert (instances[i], child.data, child.size); 1926+ if (child.data != NULL) /* could be NULL if element is non-normal */ 1927+ random_instance_assert (instances[i], child.data, child.size); 1928 g_variant_type_info_unref (child.type_info); 1929 } 1930 1931@@ -1759,7 +1764,8 @@ test_tuple (void) 1932 1933 child = g_variant_serialised_get_child (serialised, i); 1934 g_assert_true (child.type_info == instances[i]->type_info); 1935- random_instance_assert (instances[i], child.data, child.size); 1936+ if (child.data != NULL) /* could be NULL if element is non-normal */ 1937+ random_instance_assert (instances[i], child.data, child.size); 1938 g_variant_type_info_unref (child.type_info); 1939 } 1940 1941-- 1942GitLab 1943 1944 1945From 35dee77ed887a9b4d24b7a0e45f237f813e52591 Mon Sep 17 00:00:00 2001 1946From: Philip Withnall <pwithnall@endlessos.org> 1947Date: Mon, 24 Oct 2022 18:14:57 +0100 1948Subject: [PATCH 11/18] gvariant: Clarify the docs for 1949 g_variant_get_normal_form() 1950 1951Document how non-normal parts of the `GVariant` are handled. 1952 1953Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 1954--- 1955 glib/gvariant.c | 4 +++- 1956 1 file changed, 3 insertions(+), 1 deletion(-) 1957 1958diff --git a/glib/gvariant.c b/glib/gvariant.c 1959index fd066f1732..a98f10b57a 100644 1960--- a/glib/gvariant.c 1961+++ b/glib/gvariant.c 1962@@ -5936,7 +5936,9 @@ g_variant_deep_copy (GVariant *value) 1963 * marked as trusted and a new reference to it is returned. 1964 * 1965 * If @value is found not to be in normal form then a new trusted 1966- * #GVariant is created with the same value as @value. 1967+ * #GVariant is created with the same value as @value. The non-normal parts of 1968+ * @value will be replaced with default values which are guaranteed to be in 1969+ * normal form. 1970 * 1971 * It makes sense to call this function if you've received #GVariant 1972 * data from untrusted sources and you want to ensure your serialized 1973-- 1974GitLab 1975 1976 1977From e6490c84e84ba9f182fbd83b51ff4f9f5a0a1793 Mon Sep 17 00:00:00 2001 1978From: Philip Withnall <pwithnall@endlessos.org> 1979Date: Mon, 24 Oct 2022 18:43:55 +0100 1980Subject: [PATCH 12/18] gvariant: Port g_variant_deep_copy() to count its 1981 iterations directly 1982 1983This is equivalent to what `GVariantIter` does, but it means that 1984`g_variant_deep_copy()` is making its own `g_variant_get_child_value()` 1985calls. 1986 1987This will be useful in an upcoming commit, where those child values will 1988be inspected a little more deeply. 1989 1990Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 1991 1992Helps: #2121 1993--- 1994 glib/gvariant.c | 7 +++---- 1995 1 file changed, 3 insertions(+), 4 deletions(-) 1996 1997diff --git a/glib/gvariant.c b/glib/gvariant.c 1998index a98f10b57a..9334ec393e 100644 1999--- a/glib/gvariant.c 2000+++ b/glib/gvariant.c 2001@@ -5863,14 +5863,13 @@ g_variant_deep_copy (GVariant *value) 2002 case G_VARIANT_CLASS_VARIANT: 2003 { 2004 GVariantBuilder builder; 2005- GVariantIter iter; 2006- GVariant *child; 2007+ gsize i, n_children; 2008 2009 g_variant_builder_init (&builder, g_variant_get_type (value)); 2010- g_variant_iter_init (&iter, value); 2011 2012- while ((child = g_variant_iter_next_value (&iter))) 2013+ for (i = 0, n_children = g_variant_n_children (value); i < n_children; i++) 2014 { 2015+ GVariant *child = g_variant_get_child_value (value, i); 2016 g_variant_builder_add_value (&builder, g_variant_deep_copy (child)); 2017 g_variant_unref (child); 2018 } 2019-- 2020GitLab 2021 2022 2023From 168f9b42e55053611ad473898f7afb06ce20b08a Mon Sep 17 00:00:00 2001 2024From: Philip Withnall <pwithnall@endlessos.org> 2025Date: Tue, 25 Oct 2022 13:03:22 +0100 2026Subject: [PATCH 13/18] gvariant: Add internal 2027 g_variant_maybe_get_child_value() 2028 2029This will be used in a following commit. 2030 2031Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2032 2033Helps: #2540 2034--- 2035 glib/gvariant-core.c | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2036 glib/gvariant-core.h | 3 ++ 2037 2 files changed, 71 insertions(+) 2038 2039diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 2040index f660e2d578..3fa97f91eb 100644 2041--- a/glib/gvariant-core.c 2042+++ b/glib/gvariant-core.c 2043@@ -1204,6 +1204,74 @@ g_variant_get_child_value (GVariant *value, 2044 } 2045 } 2046 2047+/** 2048+ * g_variant_maybe_get_child_value: 2049+ * @value: a container #GVariant 2050+ * @index_: the index of the child to fetch 2051+ * 2052+ * Reads a child item out of a container #GVariant instance, if it is in normal 2053+ * form. If it is not in normal form, return %NULL. 2054+ * 2055+ * This function behaves the same as g_variant_get_child_value(), except that it 2056+ * returns %NULL if the child is not in normal form. g_variant_get_child_value() 2057+ * would instead return a new default value of the correct type. 2058+ * 2059+ * This is intended to be used internally to avoid unnecessary #GVariant 2060+ * allocations. 2061+ * 2062+ * The returned value is never floating. You should free it with 2063+ * g_variant_unref() when you're done with it. 2064+ * 2065+ * This function is O(1). 2066+ * 2067+ * Returns: (transfer full): the child at the specified index 2068+ * 2069+ * Since: 2.74 2070+ */ 2071+GVariant * 2072+g_variant_maybe_get_child_value (GVariant *value, 2073+ gsize index_) 2074+{ 2075+ g_return_val_if_fail (index_ < g_variant_n_children (value), NULL); 2076+ g_return_val_if_fail (value->depth < G_MAXSIZE, NULL); 2077+ 2078+ if (~g_atomic_int_get (&value->state) & STATE_SERIALISED) 2079+ { 2080+ g_variant_lock (value); 2081+ 2082+ if (~value->state & STATE_SERIALISED) 2083+ { 2084+ GVariant *child; 2085+ 2086+ child = g_variant_ref (value->contents.tree.children[index_]); 2087+ g_variant_unlock (value); 2088+ 2089+ return child; 2090+ } 2091+ 2092+ g_variant_unlock (value); 2093+ } 2094+ 2095+ { 2096+ GVariantSerialised serialised = g_variant_to_serialised (value); 2097+ GVariantSerialised s_child; 2098+ 2099+ /* get the serializer to extract the serialized data for the child 2100+ * from the serialized data for the container 2101+ */ 2102+ s_child = g_variant_serialised_get_child (serialised, index_); 2103+ 2104+ if (!(value->state & STATE_TRUSTED) && s_child.data == NULL) 2105+ { 2106+ g_variant_type_info_unref (s_child.type_info); 2107+ return NULL; 2108+ } 2109+ 2110+ g_variant_type_info_unref (s_child.type_info); 2111+ return g_variant_get_child_value (value, index_); 2112+ } 2113+} 2114+ 2115 /** 2116 * g_variant_store: 2117 * @value: the #GVariant to store 2118diff --git a/glib/gvariant-core.h b/glib/gvariant-core.h 2119index a1c34739a3..fb5f4bff45 100644 2120--- a/glib/gvariant-core.h 2121+++ b/glib/gvariant-core.h 2122@@ -38,4 +38,7 @@ GVariantTypeInfo * g_variant_get_type_info (GVarian 2123 2124 gsize g_variant_get_depth (GVariant *value); 2125 2126+GVariant * g_variant_maybe_get_child_value (GVariant *value, 2127+ gsize index_); 2128+ 2129 #endif /* __G_VARIANT_CORE_H__ */ 2130-- 2131GitLab 2132 2133 2134From c2dc74e2ec9b846d88ac4dd4368a9e2ab377cf94 Mon Sep 17 00:00:00 2001 2135From: Philip Withnall <pwithnall@endlessos.org> 2136Date: Tue, 25 Oct 2022 13:03:45 +0100 2137Subject: [PATCH 14/18] gvariant: Cut allocs of default values for children of 2138 non-normal arrays 2139MIME-Version: 1.0 2140Content-Type: text/plain; charset=UTF-8 2141Content-Transfer-Encoding: 8bit 2142 2143This improves a slow case in `g_variant_get_normal_form()` where 2144allocating many identical default values for the children of a 2145variable-sized array which has a malformed offset table would take a lot 2146of time. 2147 2148The fix is to make all child values after the first invalid one be 2149references to the default value emitted for the first invalid one, 2150rather than identical new `GVariant`s. 2151 2152In particular, this fixes a case where an attacker could create an array 2153of length L of very large tuples of size T each, corrupt the offset table 2154so they don’t have to specify the array content, and then induce 2155`g_variant_get_normal_form()` into allocating L×T default values from an 2156input which is significantly smaller than L×T in length. 2157 2158A pre-existing workaround for this issue is for code to call 2159`g_variant_is_normal_form()` before calling 2160`g_variant_get_normal_form()`, and to skip the latter call if the former 2161returns false. This commit improves the behaviour in the case that 2162`g_variant_get_normal_form()` is called anyway. 2163 2164This fix changes the time to run the `fuzz_variant_binary` test on the 2165testcase from oss-fuzz#19777 from >60s (before being terminated) with 21662.3GB of memory usage and 580k page faults; to 32s, 8.3MB of memory 2167usage and 1500 page faults (as measured by `time -v`). 2168 2169Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2170 2171Fixes: #2540 2172oss-fuzz#19777 2173--- 2174 glib/gvariant.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++- 2175 1 file changed, 65 insertions(+), 1 deletion(-) 2176 2177diff --git a/glib/gvariant.c b/glib/gvariant.c 2178index 9334ec393e..356e199a0f 100644 2179--- a/glib/gvariant.c 2180+++ b/glib/gvariant.c 2181@@ -5857,7 +5857,6 @@ g_variant_deep_copy (GVariant *value) 2182 switch (g_variant_classify (value)) 2183 { 2184 case G_VARIANT_CLASS_MAYBE: 2185- case G_VARIANT_CLASS_ARRAY: 2186 case G_VARIANT_CLASS_TUPLE: 2187 case G_VARIANT_CLASS_DICT_ENTRY: 2188 case G_VARIANT_CLASS_VARIANT: 2189@@ -5877,6 +5876,71 @@ g_variant_deep_copy (GVariant *value) 2190 return g_variant_builder_end (&builder); 2191 } 2192 2193+ case G_VARIANT_CLASS_ARRAY: 2194+ { 2195+ GVariantBuilder builder; 2196+ gsize i, n_children; 2197+ GVariant *first_invalid_child_deep_copy = NULL; 2198+ 2199+ /* Arrays are in theory treated the same as maybes, tuples, dict entries 2200+ * and variants, and could be another case in the above block of code. 2201+ * 2202+ * However, they have the property that when dealing with non-normal 2203+ * data (which is the only time g_variant_deep_copy() is currently 2204+ * called) in a variable-sized array, the code above can easily end up 2205+ * creating many default child values in order to return an array which 2206+ * is of the right length and type, but without containing non-normal 2207+ * data. This can happen if the offset table for the array is malformed. 2208+ * 2209+ * In this case, the code above would end up allocating the same default 2210+ * value for each one of the child indexes beyond the first malformed 2211+ * entry in the offset table. This can end up being a lot of identical 2212+ * allocations of default values, particularly if the non-normal array 2213+ * is crafted maliciously. 2214+ * 2215+ * Avoid that problem by returning a new reference to the same default 2216+ * value for every child after the first invalid one. This results in 2217+ * returning an equivalent array, in normal form and trusted — but with 2218+ * significantly fewer memory allocations. 2219+ * 2220+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2540 */ 2221+ 2222+ g_variant_builder_init (&builder, g_variant_get_type (value)); 2223+ 2224+ for (i = 0, n_children = g_variant_n_children (value); i < n_children; i++) 2225+ { 2226+ /* Try maybe_get_child_value() first; if it returns NULL, this child 2227+ * is non-normal. get_child_value() would have constructed and 2228+ * returned a default value in that case. */ 2229+ GVariant *child = g_variant_maybe_get_child_value (value, i); 2230+ 2231+ if (child != NULL) 2232+ { 2233+ /* Non-normal children may not always be contiguous, as they may 2234+ * be non-normal for reasons other than invalid offset table 2235+ * entries. As they are all the same type, they will all have 2236+ * the same default value though, so keep that around. */ 2237+ g_variant_builder_add_value (&builder, g_variant_deep_copy (child)); 2238+ } 2239+ else if (child == NULL && first_invalid_child_deep_copy != NULL) 2240+ { 2241+ g_variant_builder_add_value (&builder, first_invalid_child_deep_copy); 2242+ } 2243+ else if (child == NULL) 2244+ { 2245+ child = g_variant_get_child_value (value, i); 2246+ first_invalid_child_deep_copy = g_variant_ref_sink (g_variant_deep_copy (child)); 2247+ g_variant_builder_add_value (&builder, first_invalid_child_deep_copy); 2248+ } 2249+ 2250+ g_clear_pointer (&child, g_variant_unref); 2251+ } 2252+ 2253+ g_clear_pointer (&first_invalid_child_deep_copy, g_variant_unref); 2254+ 2255+ return g_variant_builder_end (&builder); 2256+ } 2257+ 2258 case G_VARIANT_CLASS_BOOLEAN: 2259 return g_variant_new_boolean (g_variant_get_boolean (value)); 2260 2261-- 2262GitLab 2263 2264 2265From f98c60e4eedf775fb896c89bc83c71c96224f6aa Mon Sep 17 00:00:00 2001 2266From: Philip Withnall <pwithnall@endlessos.org> 2267Date: Tue, 25 Oct 2022 18:03:56 +0100 2268Subject: [PATCH 15/18] gvariant: Fix a leak of a GVariantTypeInfo on an error 2269 handling path 2270 2271Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2272--- 2273 glib/gvariant-core.c | 1 + 2274 1 file changed, 1 insertion(+) 2275 2276diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c 2277index 3fa97f91eb..f441c4757e 100644 2278--- a/glib/gvariant-core.c 2279+++ b/glib/gvariant-core.c 2280@@ -1183,6 +1183,7 @@ g_variant_get_child_value (GVariant *value, 2281 G_VARIANT_MAX_RECURSION_DEPTH - value->depth) 2282 { 2283 g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_VARIANT)); 2284+ g_variant_type_info_unref (s_child.type_info); 2285 return g_variant_new_tuple (NULL, 0); 2286 } 2287 2288-- 2289GitLab 2290 2291 2292From 5f4485c4ff57fdefb1661531788def7ca5a47328 Mon Sep 17 00:00:00 2001 2293From: Philip Withnall <pwithnall@endlessos.org> 2294Date: Thu, 27 Oct 2022 12:00:04 +0100 2295Subject: [PATCH 16/18] gvariant-serialiser: Check offset table entry size is 2296 minimal 2297 2298The entries in an offset table (which is used for variable sized arrays 2299and tuples containing variable sized members) are sized so that they can 2300address every byte in the overall variant. 2301 2302The specification requires that for a variant to be in normal form, its 2303offset table entries must be the minimum width such that they can 2304address every byte in the variant. 2305 2306That minimality requirement was not checked in 2307`g_variant_is_normal_form()`, leading to two different byte arrays being 2308interpreted as the normal form of a given variant tree. That kind of 2309confusion could potentially be exploited, and is certainly a bug. 2310 2311Fix it by adding the necessary checks on offset table entry width, and 2312unit tests. 2313 2314Spotted by William Manley. 2315 2316Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2317 2318Fixes: #2794 2319--- 2320 glib/gvariant-serialiser.c | 19 +++- 2321 glib/tests/gvariant.c | 176 +++++++++++++++++++++++++++++++++++++ 2322 2 files changed, 194 insertions(+), 1 deletion(-) 2323 2324diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c 2325index 69baeeb395..fadefab659 100644 2326--- a/glib/gvariant-serialiser.c 2327+++ b/glib/gvariant-serialiser.c 2328@@ -696,6 +696,10 @@ gvs_variable_sized_array_get_frame_offsets (GVariantSerialised value) 2329 out.data_size = last_end; 2330 out.array = value.data + last_end; 2331 out.length = offsets_array_size / out.offset_size; 2332+ 2333+ if (out.length > 0 && gvs_calculate_total_size (last_end, out.length) != value.size) 2334+ return out; /* offset size not minimal */ 2335+ 2336 out.is_normal = TRUE; 2337 2338 return out; 2339@@ -1207,6 +1211,7 @@ gvs_tuple_is_normal (GVariantSerialised value) 2340 gsize length; 2341 gsize offset; 2342 gsize i; 2343+ gsize offset_table_size; 2344 2345 /* as per the comment in gvs_tuple_get_child() */ 2346 if G_UNLIKELY (value.data == NULL && value.size != 0) 2347@@ -1311,7 +1316,19 @@ gvs_tuple_is_normal (GVariantSerialised value) 2348 } 2349 } 2350 2351- return offset_ptr == offset; 2352+ /* @offset_ptr has been counting backwards from the end of the variant, to 2353+ * find the beginning of the offset table. @offset has been counting forwards 2354+ * from the beginning of the variant to find the end of the data. They should 2355+ * have met in the middle. */ 2356+ if (offset_ptr != offset) 2357+ return FALSE; 2358+ 2359+ offset_table_size = value.size - offset_ptr; 2360+ if (value.size > 0 && 2361+ gvs_calculate_total_size (offset, offset_table_size / offset_size) != value.size) 2362+ return FALSE; /* offset size not minimal */ 2363+ 2364+ return TRUE; 2365 } 2366 2367 /* Variants {{{2 2368diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 2369index 18aeb80f12..d4893ed407 100644 2370--- a/glib/tests/gvariant.c 2371+++ b/glib/tests/gvariant.c 2372@@ -5234,6 +5234,86 @@ test_normal_checking_array_offsets2 (void) 2373 g_variant_unref (variant); 2374 } 2375 2376+/* Test that an otherwise-valid serialised GVariant is considered non-normal if 2377+ * its offset table entries are too wide. 2378+ * 2379+ * See §2.3.6 (Framing Offsets) of the GVariant specification. */ 2380+static void 2381+test_normal_checking_array_offsets_minimal_sized (void) 2382+{ 2383+ GVariantBuilder builder; 2384+ gsize i; 2385+ GVariant *aay_constructed = NULL; 2386+ const guint8 *data = NULL; 2387+ guint8 *data_owned = NULL; 2388+ GVariant *aay_deserialised = NULL; 2389+ GVariant *aay_normalised = NULL; 2390+ 2391+ /* Construct an array of type aay, consisting of 128 elements which are each 2392+ * an empty array, i.e. `[[] * 128]`. This is chosen because the inner 2393+ * elements are variable sized (making the outer array variable sized, so it 2394+ * must have an offset table), but they are also zero-sized when serialised. 2395+ * So the serialised representation of @aay_constructed consists entirely of 2396+ * its offset table, which is entirely zeroes. 2397+ * 2398+ * The array is chosen to be 128 elements long because that means offset 2399+ * table entries which are 1 byte long. If the elements in the array were 2400+ * non-zero-sized (to the extent that the overall array is ≥256 bytes long), 2401+ * the offset table entries would end up being 2 bytes long. */ 2402+ g_variant_builder_init (&builder, G_VARIANT_TYPE ("aay")); 2403+ 2404+ for (i = 0; i < 128; i++) 2405+ g_variant_builder_add_value (&builder, g_variant_new_array (G_VARIANT_TYPE_BYTE, NULL, 0)); 2406+ 2407+ aay_constructed = g_variant_builder_end (&builder); 2408+ 2409+ /* Verify that the constructed array is in normal form, and its serialised 2410+ * form is `b'\0' * 128`. */ 2411+ g_assert_true (g_variant_is_normal_form (aay_constructed)); 2412+ g_assert_cmpuint (g_variant_n_children (aay_constructed), ==, 128); 2413+ g_assert_cmpuint (g_variant_get_size (aay_constructed), ==, 128); 2414+ 2415+ data = g_variant_get_data (aay_constructed); 2416+ for (i = 0; i < g_variant_get_size (aay_constructed); i++) 2417+ g_assert_cmpuint (data[i], ==, 0); 2418+ 2419+ /* Construct a serialised `aay` GVariant which is `b'\0' * 256`. This has to 2420+ * be a non-normal form of `[[] * 128]`, with 2-byte-long offset table 2421+ * entries, because each offset table entry has to be able to reference all of 2422+ * the byte boundaries in the container. All the entries in the offset table 2423+ * are zero, so all the elements of the array are zero-sized. */ 2424+ data = data_owned = g_malloc0 (256); 2425+ aay_deserialised = g_variant_new_from_data (G_VARIANT_TYPE ("aay"), 2426+ data, 2427+ 256, 2428+ FALSE, 2429+ g_free, 2430+ g_steal_pointer (&data_owned)); 2431+ 2432+ g_assert_false (g_variant_is_normal_form (aay_deserialised)); 2433+ g_assert_cmpuint (g_variant_n_children (aay_deserialised), ==, 128); 2434+ g_assert_cmpuint (g_variant_get_size (aay_deserialised), ==, 256); 2435+ 2436+ data = g_variant_get_data (aay_deserialised); 2437+ for (i = 0; i < g_variant_get_size (aay_deserialised); i++) 2438+ g_assert_cmpuint (data[i], ==, 0); 2439+ 2440+ /* Get its normal form. That should change the serialised size. */ 2441+ aay_normalised = g_variant_get_normal_form (aay_deserialised); 2442+ 2443+ g_assert_true (g_variant_is_normal_form (aay_normalised)); 2444+ g_assert_cmpuint (g_variant_n_children (aay_normalised), ==, 128); 2445+ g_assert_cmpuint (g_variant_get_size (aay_normalised), ==, 128); 2446+ 2447+ data = g_variant_get_data (aay_normalised); 2448+ for (i = 0; i < g_variant_get_size (aay_normalised); i++) 2449+ g_assert_cmpuint (data[i], ==, 0); 2450+ 2451+ g_variant_unref (aay_normalised); 2452+ g_variant_unref (aay_deserialised); 2453+ g_variant_unref (aay_constructed); 2454+} 2455+ 2456 /* Test that a tuple with invalidly large values in its offset table is 2457 * normalised successfully without looping infinitely. */ 2458 static void 2459@@ -5428,6 +5508,98 @@ test_normal_checking_tuple_offsets4 (void) 2460 g_variant_unref (variant); 2461 } 2462 2463+/* Test that an otherwise-valid serialised GVariant is considered non-normal if 2464+ * its offset table entries are too wide. 2465+ * 2466+ * See §2.3.6 (Framing Offsets) of the GVariant specification. */ 2467+static void 2468+test_normal_checking_tuple_offsets_minimal_sized (void) 2469+{ 2470+ GString *type_string = NULL; 2471+ GVariantBuilder builder; 2472+ gsize i; 2473+ GVariant *ray_constructed = NULL; 2474+ const guint8 *data = NULL; 2475+ guint8 *data_owned = NULL; 2476+ GVariant *ray_deserialised = NULL; 2477+ GVariant *ray_normalised = NULL; 2478+ 2479+ /* Construct a tuple of type (ay…ay), consisting of 129 members which are each 2480+ * an empty array, i.e. `([] * 129)`. This is chosen because the inner 2481+ * members are variable sized, so the outer tuple must have an offset table, 2482+ * but they are also zero-sized when serialised. So the serialised 2483+ * representation of @ray_constructed consists entirely of its offset table, 2484+ * which is entirely zeroes. 2485+ * 2486+ * The tuple is chosen to be 129 members long because that means it has 128 2487+ * offset table entries which are 1 byte long each. If the members in the 2488+ * tuple were non-zero-sized (to the extent that the overall tuple is ≥256 2489+ * bytes long), the offset table entries would end up being 2 bytes long. 2490+ * 2491+ * 129 members are used unlike 128 array elements in 2492+ * test_normal_checking_array_offsets_minimal_sized(), because the last member 2493+ * in a tuple never needs an offset table entry. */ 2494+ type_string = g_string_new (""); 2495+ g_string_append_c (type_string, '('); 2496+ for (i = 0; i < 129; i++) 2497+ g_string_append (type_string, "ay"); 2498+ g_string_append_c (type_string, ')'); 2499+ 2500+ g_variant_builder_init (&builder, G_VARIANT_TYPE (type_string->str)); 2501+ 2502+ for (i = 0; i < 129; i++) 2503+ g_variant_builder_add_value (&builder, g_variant_new_array (G_VARIANT_TYPE_BYTE, NULL, 0)); 2504+ 2505+ ray_constructed = g_variant_builder_end (&builder); 2506+ 2507+ /* Verify that the constructed tuple is in normal form, and its serialised 2508+ * form is `b'\0' * 128`. */ 2509+ g_assert_true (g_variant_is_normal_form (ray_constructed)); 2510+ g_assert_cmpuint (g_variant_n_children (ray_constructed), ==, 129); 2511+ g_assert_cmpuint (g_variant_get_size (ray_constructed), ==, 128); 2512+ 2513+ data = g_variant_get_data (ray_constructed); 2514+ for (i = 0; i < g_variant_get_size (ray_constructed); i++) 2515+ g_assert_cmpuint (data[i], ==, 0); 2516+ 2517+ /* Construct a serialised `(ay…ay)` GVariant which is `b'\0' * 256`. This has 2518+ * to be a non-normal form of `([] * 129)`, with 2-byte-long offset table 2519+ * entries, because each offset table entry has to be able to reference all of 2520+ * the byte boundaries in the container. All the entries in the offset table 2521+ * are zero, so all the members of the tuple are zero-sized. */ 2522+ data = data_owned = g_malloc0 (256); 2523+ ray_deserialised = g_variant_new_from_data (G_VARIANT_TYPE (type_string->str), 2524+ data, 2525+ 256, 2526+ FALSE, 2527+ g_free, 2528+ g_steal_pointer (&data_owned)); 2529+ 2530+ g_assert_false (g_variant_is_normal_form (ray_deserialised)); 2531+ g_assert_cmpuint (g_variant_n_children (ray_deserialised), ==, 129); 2532+ g_assert_cmpuint (g_variant_get_size (ray_deserialised), ==, 256); 2533+ 2534+ data = g_variant_get_data (ray_deserialised); 2535+ for (i = 0; i < g_variant_get_size (ray_deserialised); i++) 2536+ g_assert_cmpuint (data[i], ==, 0); 2537+ 2538+ /* Get its normal form. That should change the serialised size. */ 2539+ ray_normalised = g_variant_get_normal_form (ray_deserialised); 2540+ 2541+ g_assert_true (g_variant_is_normal_form (ray_normalised)); 2542+ g_assert_cmpuint (g_variant_n_children (ray_normalised), ==, 129); 2543+ g_assert_cmpuint (g_variant_get_size (ray_normalised), ==, 128); 2544+ 2545+ data = g_variant_get_data (ray_normalised); 2546+ for (i = 0; i < g_variant_get_size (ray_normalised); i++) 2547+ g_assert_cmpuint (data[i], ==, 0); 2548+ 2549+ g_variant_unref (ray_normalised); 2550+ g_variant_unref (ray_deserialised); 2551+ g_variant_unref (ray_constructed); 2552+ g_string_free (type_string, TRUE); 2553+} 2554+ 2555 /* Test that an empty object path is normalised successfully to the base object 2556 * path, ‘/’. */ 2557 static void 2558@@ -5576,6 +5748,8 @@ main (int argc, char **argv) 2559 test_normal_checking_array_offsets); 2560 g_test_add_func ("/gvariant/normal-checking/array-offsets2", 2561 test_normal_checking_array_offsets2); 2562+ g_test_add_func ("/gvariant/normal-checking/array-offsets/minimal-sized", 2563+ test_normal_checking_array_offsets_minimal_sized); 2564 g_test_add_func ("/gvariant/normal-checking/tuple-offsets", 2565 test_normal_checking_tuple_offsets); 2566 g_test_add_func ("/gvariant/normal-checking/tuple-offsets2", 2567@@ -5584,6 +5758,8 @@ main (int argc, char **argv) 2568 test_normal_checking_tuple_offsets3); 2569 g_test_add_func ("/gvariant/normal-checking/tuple-offsets4", 2570 test_normal_checking_tuple_offsets4); 2571+ g_test_add_func ("/gvariant/normal-checking/tuple-offsets/minimal-sized", 2572+ test_normal_checking_tuple_offsets_minimal_sized); 2573 g_test_add_func ("/gvariant/normal-checking/empty-object-path", 2574 test_normal_checking_empty_object_path); 2575 2576-- 2577GitLab 2578 2579 2580From 4c4cf568f0f710baf0bd04d52df715636bc6b971 Mon Sep 17 00:00:00 2001 2581From: Philip Withnall <pwithnall@endlessos.org> 2582Date: Thu, 27 Oct 2022 16:13:54 +0100 2583Subject: [PATCH 17/18] gvariant: Fix g_variant_byteswap() returning non-normal 2584 data sometimes 2585MIME-Version: 1.0 2586Content-Type: text/plain; charset=UTF-8 2587Content-Transfer-Encoding: 8bit 2588 2589If `g_variant_byteswap()` was called on a non-normal variant of a type 2590which doesn’t need byteswapping, it would return a non-normal output. 2591 2592That contradicts the documentation, which says that the return value is 2593always in normal form. 2594 2595Fix the code so it matches the documentation. 2596 2597Includes a unit test. 2598 2599Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2600 2601Helps: #2797 2602--- 2603 glib/gvariant.c | 8 +++++--- 2604 glib/tests/gvariant.c | 24 ++++++++++++++++++++++++ 2605 2 files changed, 29 insertions(+), 3 deletions(-) 2606 2607diff --git a/glib/gvariant.c b/glib/gvariant.c 2608index 356e199a0f..f7b2e97036 100644 2609--- a/glib/gvariant.c 2610+++ b/glib/gvariant.c 2611@@ -6084,14 +6084,16 @@ g_variant_byteswap (GVariant *value) 2612 g_variant_serialised_byteswap (serialised); 2613 2614 bytes = g_bytes_new_take (serialised.data, serialised.size); 2615- new = g_variant_new_from_bytes (g_variant_get_type (value), bytes, TRUE); 2616+ new = g_variant_ref_sink (g_variant_new_from_bytes (g_variant_get_type (value), bytes, TRUE)); 2617 g_bytes_unref (bytes); 2618 } 2619 else 2620 /* contains no multi-byte data */ 2621- new = value; 2622+ new = g_variant_get_normal_form (value); 2623 2624- return g_variant_ref_sink (new); 2625+ g_assert (g_variant_is_trusted (new)); 2626+ 2627+ return g_steal_pointer (&new); 2628 } 2629 2630 /** 2631diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 2632index d4893ed407..1e7ed16cc9 100644 2633--- a/glib/tests/gvariant.c 2634+++ b/glib/tests/gvariant.c 2635@@ -3826,6 +3826,29 @@ test_gv_byteswap (void) 2636 g_free (string); 2637 } 2638 2639+static void 2640+test_gv_byteswap_non_normal_non_aligned (void) 2641+{ 2642+ const guint8 data[] = { 0x02 }; 2643+ GVariant *v = NULL; 2644+ GVariant *v_byteswapped = NULL; 2645+ 2646+ g_test_summary ("Test that calling g_variant_byteswap() on a variant which " 2647+ "is in non-normal form and doesn’t need byteswapping returns " 2648+ "the same variant in normal form."); 2649+ 2650+ v = g_variant_new_from_data (G_VARIANT_TYPE_BOOLEAN, data, sizeof (data), FALSE, NULL, NULL); 2651+ g_assert_false (g_variant_is_normal_form (v)); 2652+ 2653+ v_byteswapped = g_variant_byteswap (v); 2654+ g_assert_true (g_variant_is_normal_form (v_byteswapped)); 2655+ 2656+ g_assert_cmpvariant (v, v_byteswapped); 2657+ 2658+ g_variant_unref (v); 2659+ g_variant_unref (v_byteswapped); 2660+} 2661+ 2662 static void 2663 test_parser (void) 2664 { 2665@@ -5711,6 +5734,7 @@ main (int argc, char **argv) 2666 g_test_add_func ("/gvariant/builder-memory", test_builder_memory); 2667 g_test_add_func ("/gvariant/hashing", test_hashing); 2668 g_test_add_func ("/gvariant/byteswap", test_gv_byteswap); 2669+ g_test_add_func ("/gvariant/byteswap/non-normal-non-aligned", test_gv_byteswap_non_normal_non_aligned); 2670 g_test_add_func ("/gvariant/parser", test_parses); 2671 g_test_add_func ("/gvariant/parser/integer-bounds", test_parser_integer_bounds); 2672 g_test_add_func ("/gvariant/parser/recursion", test_parser_recursion); 2673-- 2674GitLab 2675 2676 2677From a70a16b28bf78403dc86ae798c291ba167573d4a Mon Sep 17 00:00:00 2001 2678From: Philip Withnall <pwithnall@endlessos.org> 2679Date: Thu, 27 Oct 2022 22:53:13 +0100 2680Subject: [PATCH 18/18] gvariant: Allow g_variant_byteswap() to operate on 2681 tree-form variants 2682MIME-Version: 1.0 2683Content-Type: text/plain; charset=UTF-8 2684Content-Transfer-Encoding: 8bit 2685 2686This avoids needing to always serialise a variant before byteswapping it. 2687With variants in non-normal forms, serialisation can result in a large 2688increase in size of the variant, and a lot of allocations for leaf 2689`GVariant`s. This can lead to a denial of service attack. 2690 2691Avoid that by changing byteswapping so that it happens on the tree form 2692of the variant if the input is in non-normal form. If the input is in 2693normal form (either serialised or in tree form), continue using the 2694existing code as byteswapping an already-serialised normal variant is 2695about 3× faster than byteswapping on the equivalent tree form. 2696 2697The existing unit tests cover byteswapping well, but need some 2698adaptation so that they operate on tree form variants too. 2699 2700I considered dropping the serialised byteswapping code and doing all 2701byteswapping on tree-form variants, as that would make maintenance 2702simpler (avoiding having two parallel implementations of byteswapping). 2703However, most inputs to `g_variant_byteswap()` are likely to be 2704serialised variants (coming from a byte array of input from some foreign 2705source) and most of them are going to be in normal form (as corruption 2706and malicious action are rare). So getting rid of the serialised 2707byteswapping code would impose quite a performance penalty on the common 2708case. 2709 2710Signed-off-by: Philip Withnall <pwithnall@endlessos.org> 2711 2712Fixes: #2797 2713--- 2714 glib/gvariant.c | 87 ++++++++++++++++++++++++++++++++----------- 2715 glib/tests/gvariant.c | 57 ++++++++++++++++++++++++---- 2716 2 files changed, 115 insertions(+), 29 deletions(-) 2717 2718diff --git a/glib/gvariant.c b/glib/gvariant.c 2719index f7b2e97036..5fef88d58d 100644 2720--- a/glib/gvariant.c 2721+++ b/glib/gvariant.c 2722@@ -5852,7 +5852,8 @@ g_variant_iter_loop (GVariantIter *iter, 2723 2724 /* Serialized data {{{1 */ 2725 static GVariant * 2726-g_variant_deep_copy (GVariant *value) 2727+g_variant_deep_copy (GVariant *value, 2728+ gboolean byteswap) 2729 { 2730 switch (g_variant_classify (value)) 2731 { 2732@@ -5869,7 +5870,7 @@ g_variant_deep_copy (GVariant *value) 2733 for (i = 0, n_children = g_variant_n_children (value); i < n_children; i++) 2734 { 2735 GVariant *child = g_variant_get_child_value (value, i); 2736- g_variant_builder_add_value (&builder, g_variant_deep_copy (child)); 2737+ g_variant_builder_add_value (&builder, g_variant_deep_copy (child, byteswap)); 2738 g_variant_unref (child); 2739 } 2740 2741@@ -5920,7 +5921,7 @@ g_variant_deep_copy (GVariant *value) 2742 * be non-normal for reasons other than invalid offset table 2743 * entries. As they are all the same type, they will all have 2744 * the same default value though, so keep that around. */ 2745- g_variant_builder_add_value (&builder, g_variant_deep_copy (child)); 2746+ g_variant_builder_add_value (&builder, g_variant_deep_copy (child, byteswap)); 2747 } 2748 else if (child == NULL && first_invalid_child_deep_copy != NULL) 2749 { 2750@@ -5929,7 +5930,7 @@ g_variant_deep_copy (GVariant *value) 2751 else if (child == NULL) 2752 { 2753 child = g_variant_get_child_value (value, i); 2754- first_invalid_child_deep_copy = g_variant_ref_sink (g_variant_deep_copy (child)); 2755+ first_invalid_child_deep_copy = g_variant_ref_sink (g_variant_deep_copy (child, byteswap)); 2756 g_variant_builder_add_value (&builder, first_invalid_child_deep_copy); 2757 } 2758 2759@@ -5948,28 +5949,63 @@ g_variant_deep_copy (GVariant *value) 2760 return g_variant_new_byte (g_variant_get_byte (value)); 2761 2762 case G_VARIANT_CLASS_INT16: 2763- return g_variant_new_int16 (g_variant_get_int16 (value)); 2764+ if (byteswap) 2765+ return g_variant_new_int16 (GUINT16_SWAP_LE_BE (g_variant_get_int16 (value))); 2766+ else 2767+ return g_variant_new_int16 (g_variant_get_int16 (value)); 2768 2769 case G_VARIANT_CLASS_UINT16: 2770- return g_variant_new_uint16 (g_variant_get_uint16 (value)); 2771+ if (byteswap) 2772+ return g_variant_new_uint16 (GUINT16_SWAP_LE_BE (g_variant_get_uint16 (value))); 2773+ else 2774+ return g_variant_new_uint16 (g_variant_get_uint16 (value)); 2775 2776 case G_VARIANT_CLASS_INT32: 2777- return g_variant_new_int32 (g_variant_get_int32 (value)); 2778+ if (byteswap) 2779+ return g_variant_new_int32 (GUINT32_SWAP_LE_BE (g_variant_get_int32 (value))); 2780+ else 2781+ return g_variant_new_int32 (g_variant_get_int32 (value)); 2782 2783 case G_VARIANT_CLASS_UINT32: 2784- return g_variant_new_uint32 (g_variant_get_uint32 (value)); 2785+ if (byteswap) 2786+ return g_variant_new_uint32 (GUINT32_SWAP_LE_BE (g_variant_get_uint32 (value))); 2787+ else 2788+ return g_variant_new_uint32 (g_variant_get_uint32 (value)); 2789 2790 case G_VARIANT_CLASS_INT64: 2791- return g_variant_new_int64 (g_variant_get_int64 (value)); 2792+ if (byteswap) 2793+ return g_variant_new_int64 (GUINT64_SWAP_LE_BE (g_variant_get_int64 (value))); 2794+ else 2795+ return g_variant_new_int64 (g_variant_get_int64 (value)); 2796 2797 case G_VARIANT_CLASS_UINT64: 2798- return g_variant_new_uint64 (g_variant_get_uint64 (value)); 2799+ if (byteswap) 2800+ return g_variant_new_uint64 (GUINT64_SWAP_LE_BE (g_variant_get_uint64 (value))); 2801+ else 2802+ return g_variant_new_uint64 (g_variant_get_uint64 (value)); 2803 2804 case G_VARIANT_CLASS_HANDLE: 2805- return g_variant_new_handle (g_variant_get_handle (value)); 2806+ if (byteswap) 2807+ return g_variant_new_handle (GUINT32_SWAP_LE_BE (g_variant_get_handle (value))); 2808+ else 2809+ return g_variant_new_handle (g_variant_get_handle (value)); 2810 2811 case G_VARIANT_CLASS_DOUBLE: 2812- return g_variant_new_double (g_variant_get_double (value)); 2813+ if (byteswap) 2814+ { 2815+ /* We have to convert the double to a uint64 here using a union, 2816+ * because a cast will round it numerically. */ 2817+ union 2818+ { 2819+ guint64 u64; 2820+ gdouble dbl; 2821+ } u1, u2; 2822+ u1.dbl = g_variant_get_double (value); 2823+ u2.u64 = GUINT64_SWAP_LE_BE (u1.u64); 2824+ return g_variant_new_double (u2.dbl); 2825+ } 2826+ else 2827+ return g_variant_new_double (g_variant_get_double (value)); 2828 2829 case G_VARIANT_CLASS_STRING: 2830 return g_variant_new_string (g_variant_get_string (value, NULL)); 2831@@ -6026,7 +6062,7 @@ g_variant_get_normal_form (GVariant *value) 2832 if (g_variant_is_normal_form (value)) 2833 return g_variant_ref (value); 2834 2835- trusted = g_variant_deep_copy (value); 2836+ trusted = g_variant_deep_copy (value, FALSE); 2837 g_assert (g_variant_is_trusted (trusted)); 2838 2839 return g_variant_ref_sink (trusted); 2840@@ -6046,6 +6082,11 @@ g_variant_get_normal_form (GVariant *value) 2841 * contain multi-byte numeric data. That include strings, booleans, 2842 * bytes and containers containing only these things (recursively). 2843 * 2844+ * While this function can safely handle untrusted, non-normal data, it is 2845+ * recommended to check whether the input is in normal form beforehand, using 2846+ * g_variant_is_normal_form(), and to reject non-normal inputs if your 2847+ * application can be strict about what inputs it rejects. 2848+ * 2849 * The returned value is always in normal form and is marked as trusted. 2850 * 2851 * Returns: (transfer full): the byteswapped form of @value 2852@@ -6064,22 +6105,21 @@ g_variant_byteswap (GVariant *value) 2853 2854 g_variant_type_info_query (type_info, &alignment, NULL); 2855 2856- if (alignment) 2857- /* (potentially) contains multi-byte numeric data */ 2858+ if (alignment && g_variant_is_normal_form (value)) 2859 { 2860+ /* (potentially) contains multi-byte numeric data, but is also already in 2861+ * normal form so we can use a faster byteswapping codepath on the 2862+ * serialised data */ 2863 GVariantSerialised serialised = { 0, }; 2864- GVariant *trusted; 2865 GBytes *bytes; 2866 2867- trusted = g_variant_get_normal_form (value); 2868- serialised.type_info = g_variant_get_type_info (trusted); 2869- serialised.size = g_variant_get_size (trusted); 2870+ serialised.type_info = g_variant_get_type_info (value); 2871+ serialised.size = g_variant_get_size (value); 2872 serialised.data = g_malloc (serialised.size); 2873- serialised.depth = g_variant_get_depth (trusted); 2874+ serialised.depth = g_variant_get_depth (value); 2875 serialised.ordered_offsets_up_to = G_MAXSIZE; /* operating on the normal form */ 2876 serialised.checked_offsets_up_to = G_MAXSIZE; 2877- g_variant_store (trusted, serialised.data); 2878- g_variant_unref (trusted); 2879+ g_variant_store (value, serialised.data); 2880 2881 g_variant_serialised_byteswap (serialised); 2882 2883@@ -6087,6 +6127,9 @@ g_variant_byteswap (GVariant *value) 2884 new = g_variant_ref_sink (g_variant_new_from_bytes (g_variant_get_type (value), bytes, TRUE)); 2885 g_bytes_unref (bytes); 2886 } 2887+ else if (alignment) 2888+ /* (potentially) contains multi-byte numeric data */ 2889+ new = g_variant_ref_sink (g_variant_deep_copy (value, TRUE)); 2890 else 2891 /* contains no multi-byte data */ 2892 new = g_variant_get_normal_form (value); 2893diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c 2894index 1e7ed16cc9..3357488a4f 100644 2895--- a/glib/tests/gvariant.c 2896+++ b/glib/tests/gvariant.c 2897@@ -2288,24 +2288,67 @@ serialise_tree (TreeInstance *tree, 2898 static void 2899 test_byteswap (void) 2900 { 2901- GVariantSerialised one = { 0, }, two = { 0, }; 2902+ GVariantSerialised one = { 0, }, two = { 0, }, three = { 0, }; 2903 TreeInstance *tree; 2904- 2905+ GVariant *one_variant = NULL; 2906+ GVariant *two_variant = NULL; 2907+ GVariant *two_byteswapped = NULL; 2908+ GVariant *three_variant = NULL; 2909+ GVariant *three_byteswapped = NULL; 2910+ guint8 *three_data_copy = NULL; 2911+ gsize three_size_copy = 0; 2912+ 2913+ /* Write a tree out twice, once normally and once byteswapped. */ 2914 tree = tree_instance_new (NULL, 3); 2915 serialise_tree (tree, &one); 2916 2917+ one_variant = g_variant_new_from_data (G_VARIANT_TYPE (g_variant_type_info_get_type_string (one.type_info)), 2918+ one.data, one.size, FALSE, NULL, NULL); 2919+ 2920 i_am_writing_byteswapped = TRUE; 2921 serialise_tree (tree, &two); 2922+ serialise_tree (tree, &three); 2923 i_am_writing_byteswapped = FALSE; 2924 2925- g_variant_serialised_byteswap (two); 2926- 2927- g_assert_cmpmem (one.data, one.size, two.data, two.size); 2928- g_assert_cmpuint (one.depth, ==, two.depth); 2929- 2930+ /* Swap the first byteswapped one back using the function we want to test. */ 2931+ two_variant = g_variant_new_from_data (G_VARIANT_TYPE (g_variant_type_info_get_type_string (two.type_info)), 2932+ two.data, two.size, FALSE, NULL, NULL); 2933+ two_byteswapped = g_variant_byteswap (two_variant); 2934+ 2935+ /* Make the second byteswapped one non-normal (hopefully), and then byteswap 2936+ * it back using the function we want to test in its non-normal mode. 2937+ * This might not work because it’s not necessarily possible to make an 2938+ * arbitrary random variant non-normal. Adding a single zero byte to the end 2939+ * often makes something non-normal but still readable. */ 2940+ three_size_copy = three.size + 1; 2941+ three_data_copy = g_malloc (three_size_copy); 2942+ memcpy (three_data_copy, three.data, three.size); 2943+ three_data_copy[three.size] = '\0'; 2944+ 2945+ three_variant = g_variant_new_from_data (G_VARIANT_TYPE (g_variant_type_info_get_type_string (three.type_info)), 2946+ three_data_copy, three_size_copy, FALSE, NULL, NULL); 2947+ three_byteswapped = g_variant_byteswap (three_variant); 2948+ 2949+ /* Check they’re the same. We can always compare @one_variant and 2950+ * @two_byteswapped. We can only compare @two_byteswapped and 2951+ * @three_byteswapped if @two_variant and @three_variant are equal: in that 2952+ * case, the corruption to @three_variant was enough to make it non-normal but 2953+ * not enough to change its value. */ 2954+ g_assert_cmpvariant (one_variant, two_byteswapped); 2955+ 2956+ if (g_variant_equal (two_variant, three_variant)) 2957+ g_assert_cmpvariant (two_byteswapped, three_byteswapped); 2958+ 2959+ g_variant_unref (three_byteswapped); 2960+ g_variant_unref (three_variant); 2961+ g_variant_unref (two_byteswapped); 2962+ g_variant_unref (two_variant); 2963+ g_variant_unref (one_variant); 2964 tree_instance_free (tree); 2965 g_free (one.data); 2966 g_free (two.data); 2967+ g_free (three.data); 2968+ g_free (three_data_copy); 2969 } 2970 2971 static void 2972-- 2973GitLab 2974 2975