summaryrefslogtreecommitdiffstats
path: root/meta/recipes-core/glib-2.0/glib-2.0/CVE-2023-32665-0006.patch
blob: 8558a7911f5d3cc56bcbbc1aba52219d3a517918 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
From 7cf6f5b69146d20948d42f0c476688fe17fef787 Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@endlessos.org>
Date: Wed, 16 Aug 2023 12:09:06 +0000
Subject: [PATCH] gvariant: Don't allow child elements of a tuple to overlap
 each other

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

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

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

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

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

Fixes: #2121

CVE: CVE-2023-32665
Upstream-Status: Backport from [https://gitlab.gnome.org/GNOME/glib/-/commit/7cf6f5b69146d20948d42f0c476688fe17fef787]
Signed-off-by: Siddharth Doshi <sdoshi@mvista.com>
---
 glib/gvariant-core.c       |   6 +-
 glib/gvariant-serialiser.c |  40 ++++++++
 glib/gvariant-serialiser.h |   7 +-
 glib/gvariant.c            |   1 +
 glib/tests/gvariant.c      | 181 +++++++++++++++++++++++++++++++++++++
 5 files changed, 232 insertions(+), 3 deletions(-)

diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c
index 9b51e15..b951cd9 100644
--- a/glib/gvariant-core.c
+++ b/glib/gvariant-core.c
@@ -1,6 +1,7 @@
 /*
  * Copyright © 2007, 2008 Ryan Lortie
  * Copyright © 2010 Codethink Limited
+ * Copyright © 2022 Endless OS Foundation, LLC
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -179,7 +180,7 @@ struct _GVariant
  *                             offsets themselves.
  *
  *                             This field is only relevant for arrays of non
- *                             fixed width types.
+ *                             fixed width types and for tuples.
  *
  *   .tree: Only valid when the instance is in tree form.
  *
@@ -1117,6 +1118,9 @@ g_variant_get_child_value (GVariant *value,
      */
     s_child = g_variant_serialised_get_child (serialised, index_);
 
+    /* Update the cached ordered_offsets_up_to, since @serialised will be thrown away when this function exits */
+    value->contents.serialised.ordered_offsets_up_to = MAX (value->contents.serialised.ordered_offsets_up_to, serialised.ordered_offsets_up_to);
+
     /* Check whether this would cause nesting too deep. If so, return a fake
      * child. The only situation we expect this to happen in is with a variant,
      * as all other deeply-nested types have a static type, and hence should
diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c
index fb75923..cd4a3e6 100644
--- a/glib/gvariant-serialiser.c
+++ b/glib/gvariant-serialiser.c
@@ -942,6 +942,10 @@ gvs_variable_sized_array_is_normal (GVariantSerialised value)
  * for the tuple.  See the notes in gvarianttypeinfo.h.
  */
 
+/* Note: This doesn’t guarantee that @out_member_end >= @out_member_start; that
+ * condition may not hold true for invalid serialised variants. The caller is
+ * responsible for checking the returned values and handling invalid ones
+ * appropriately. */
 static void
 gvs_tuple_get_member_bounds (GVariantSerialised  value,
                              gsize               index_,
@@ -1028,6 +1032,42 @@ gvs_tuple_get_child (GVariantSerialised value,
       return child;
     }
 
+  /* If the requested @index_ is beyond the set of indices whose framing offsets
+   * have been checked, check the remaining offsets to see whether they’re
+   * normal (in order, no overlapping tuple elements).
+   *
+   * Unlike the checks in gvs_variable_sized_array_get_child(), we have to check
+   * all the tuple *elements* here, not just all the framing offsets, since
+   * tuples contain a mix of elements which use framing offsets and ones which
+   * don’t. None of them are allowed to overlap. */
+  if (index_ > value.ordered_offsets_up_to)
+    {
+      gsize i, prev_i_end = 0;
+
+      if (value.ordered_offsets_up_to > 0)
+        gvs_tuple_get_member_bounds (value, value.ordered_offsets_up_to - 1, offset_size, NULL, &prev_i_end);
+
+      for (i = value.ordered_offsets_up_to; i <= index_; i++)
+        {
+          gsize i_start, i_end;
+
+          gvs_tuple_get_member_bounds (value, i, offset_size, &i_start, &i_end);
+
+          if (i_start > i_end || i_start < prev_i_end || i_end > value.size)
+            break;
+
+          prev_i_end = i_end;
+        }
+
+      value.ordered_offsets_up_to = i - 1;
+    }
+
+  if (index_ > value.ordered_offsets_up_to)
+    {
+      /* Offsets are invalid somewhere, so return an empty child. */
+      return child;
+    }
+
   if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET)
     {
       if (offset_size * (member_info->i + 2) > value.size)
diff --git a/glib/gvariant-serialiser.h b/glib/gvariant-serialiser.h
index 99d18ef..144aec8 100644
--- a/glib/gvariant-serialiser.h
+++ b/glib/gvariant-serialiser.h
@@ -34,8 +34,11 @@ typedef struct
    * This guarantees that the bytes of element n don't overlap with any previous
    * element.
    *
-   * This is both read and set by g_variant_serialised_get_child for arrays of
-   * non-fixed-width types */
+   * This is both read and set by g_variant_serialised_get_child() for arrays of
+   * non-fixed-width types, and for tuples.
+   *
+   * Even when dealing with tuples, @ordered_offsets_up_to is an element index,
+   * rather than an index into the frame offsets. */
   gsize             ordered_offsets_up_to;
 } GVariantSerialised;
 
diff --git a/glib/gvariant.c b/glib/gvariant.c
index d6f68a9..cdb428e 100644
--- a/glib/gvariant.c
+++ b/glib/gvariant.c
@@ -5945,6 +5945,7 @@ g_variant_byteswap (GVariant *value)
       serialised.type_info = g_variant_get_type_info (trusted);
       serialised.size = g_variant_get_size (trusted);
       serialised.data = g_malloc (serialised.size);
+      serialised.ordered_offsets_up_to = G_MAXSIZE;  /* operating on the normal form */
       g_variant_store (trusted, serialised.data);
       g_variant_unref (trusted);
 
diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c
index 967e9a1..a84b02e 100644
--- a/glib/tests/gvariant.c
+++ b/glib/tests/gvariant.c
@@ -1,6 +1,7 @@
 /*
  * Copyright © 2010 Codethink Limited
  * Copyright © 2020 William Manley
+ * Copyright © 2022 Endless OS Foundation, LLC
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1451,6 +1452,7 @@ test_maybe (void)
         serialised.data = flavoured_malloc (needed_size, flavour);
         serialised.size = needed_size;
         serialised.depth = 0;
+	serialised.ordered_offsets_up_to = 0;
 
         g_variant_serialiser_serialise (serialised,
                                         random_instance_filler,
@@ -1574,6 +1576,7 @@ test_array (void)
         serialised.data = flavoured_malloc (needed_size, flavour);
         serialised.size = needed_size;
         serialised.depth = 0;
+	serialised.ordered_offsets_up_to = 0;
 
         g_variant_serialiser_serialise (serialised, random_instance_filler,
                                         (gpointer *) instances, n_children);
@@ -1738,6 +1741,7 @@ test_tuple (void)
         serialised.data = flavoured_malloc (needed_size, flavour);
         serialised.size = needed_size;
         serialised.depth = 0;
+	serialised.ordered_offsets_up_to = 0;
 
         g_variant_serialiser_serialise (serialised, random_instance_filler,
                                         (gpointer *) instances, n_children);
@@ -1834,6 +1838,7 @@ test_variant (void)
         serialised.data = flavoured_malloc (needed_size, flavour);
         serialised.size = needed_size;
         serialised.depth = 0;
+	serialised.ordered_offsets_up_to = 0;
 
         g_variant_serialiser_serialise (serialised, random_instance_filler,
                                         (gpointer *) &instance, 1);
@@ -5106,6 +5111,176 @@ test_normal_checking_tuple_offsets (void)
   g_variant_unref (variant);
 }
 
+/* This is a regression test that we can't have non-normal values that take up
+ * significantly more space than the normal equivalent, by specifying the
+ * offset table entries so that tuple elements overlap.
+ *
+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838503 and
+ * https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_838513 */
+static void
+test_normal_checking_tuple_offsets2 (void)
+{
+  const GVariantType *data_type = G_VARIANT_TYPE ("(yyaiyyaiyy)");
+  const guint8 data[] = {
+    0x12, 0x34, 0x56, 0x78, 0x01,
+    /*
+         ^───────────────────┘
+
+    ^^^^^^^^^^                   1st yy
+          ^^^^^^^^^^             2nd yy
+                ^^^^^^^^^^       3rd yy
+                            ^^^^ Framing offsets
+     */
+
+  /* If this variant was encoded normally, it would be something like this:
+   * 0x12, 0x34,  pad,  pad, [array bytes], 0x56, 0x78,  pad,  pad, [array bytes], 0x9A, 0xBC, 0xXX
+   *                                      ^─────────────────────────────────────────────────────┘
+   *
+   * ^^^^^^^^^^                                                                                     1st yy
+   *                                        ^^^^^^^^^^                                              2nd yy
+   *                                                                               ^^^^^^^^^^       3rd yy
+   *                                                                                           ^^^^ Framing offsets
+   */
+  };
+  gsize size = sizeof (data);
+  GVariant *variant = NULL;
+  GVariant *normal_variant = NULL;
+  GVariant *expected = NULL;
+
+  variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL);
+  g_assert_nonnull (variant);
+
+  normal_variant = g_variant_get_normal_form (variant);
+  g_assert_nonnull (normal_variant);
+  g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3);
+
+  expected = g_variant_new_parsed (
+      "@(yyaiyyaiyy) (0x12, 0x34, [], 0x00, 0x00, [], 0x00, 0x00)");
+  g_assert_cmpvariant (expected, variant);
+  g_assert_cmpvariant (expected, normal_variant);
+
+  g_variant_unref (expected);
+  g_variant_unref (normal_variant);
+  g_variant_unref (variant);
+}
+
+/* This is a regression test that overlapping entries in the offset table are
+ * decoded consistently, even though they’re non-normal.
+ *
+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */
+static void
+test_normal_checking_tuple_offsets3 (void)
+{
+  /* The expected decoding of this non-normal byte stream is complex. See
+   * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant
+   * specification.
+   *
+   * The rule “Child Values Overlapping Framing Offsets” from the specification
+   * says that the first `ay` must be decoded as `[0x01]` even though it
+   * overlaps the first byte of the offset table. However, since commit
+   * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow
+   * this as it’s exploitable. So the first `ay` must be given a default value.
+   *
+   * The second and third `ay`s must be given default values because of rule
+   * “End Boundary Precedes Start Boundary”.
+   *
+   * The `i` must be given a default value because of rule “Start or End
+   * Boundary of a Child Falls Outside the Container”.
+   */
+  const GVariantType *data_type = G_VARIANT_TYPE ("(ayayiay)");
+  const guint8 data[] = {
+    0x01, 0x00, 0x02,
+    /*
+               ^──┘
+
+    ^^^^^^^^^^                   1st ay, bytes 0-2 (but given a default value anyway, see above)
+                                 2nd ay, bytes 2-0
+                                     i,  bytes 0-4
+                                 3rd ay, bytes 4-1
+          ^^^^^^^^^^ Framing offsets
+     */
+  };
+  gsize size = sizeof (data);
+  GVariant *variant = NULL;
+  GVariant *normal_variant = NULL;
+  GVariant *expected = NULL;
+
+  variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL);
+  g_assert_nonnull (variant);
+
+  g_assert_false (g_variant_is_normal_form (variant));
+
+  normal_variant = g_variant_get_normal_form (variant);
+  g_assert_nonnull (normal_variant);
+  g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3);
+
+  expected = g_variant_new_parsed ("@(ayayiay) ([], [], 0, [])");
+  g_assert_cmpvariant (expected, variant);
+  g_assert_cmpvariant (expected, normal_variant);
+
+  g_variant_unref (expected);
+  g_variant_unref (normal_variant);
+  g_variant_unref (variant);
+}
+
+/* This is a regression test that overlapping entries in the offset table are
+ * decoded consistently, even though they’re non-normal.
+ *
+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2121#note_910935 */
+static void
+test_normal_checking_tuple_offsets4 (void)
+{
+  /* The expected decoding of this non-normal byte stream is complex. See
+   * section 2.7.3 (Handling Non-Normal Serialised Data) of the GVariant
+   * specification.
+   *
+   * The rule “Child Values Overlapping Framing Offsets” from the specification
+   * says that the first `ay` must be decoded as `[0x01]` even though it
+   * overlaps the first byte of the offset table. However, since commit
+   * 7eedcd76f7d5b8c98fa60013e1fe6e960bf19df3, GLib explicitly doesn’t allow
+   * this as it’s exploitable. So the first `ay` must be given a default value.
+   *
+   * The second `ay` must be given a default value because of rule “End Boundary
+   * Precedes Start Boundary”.
+   *
+   * The third `ay` must be given a default value because its framing offsets
+   * overlap that of the first `ay`.
+   */
+  const GVariantType *data_type = G_VARIANT_TYPE ("(ayayay)");
+  const guint8 data[] = {
+    0x01, 0x00, 0x02,
+    /*
+               ^──┘
+
+    ^^^^^^^^^^                   1st ay, bytes 0-2 (but given a default value anyway, see above)
+                                 2nd ay, bytes 2-0
+                                 3rd ay, bytes 0-1
+          ^^^^^^^^^^ Framing offsets
+     */
+  };
+  gsize size = sizeof (data);
+  GVariant *variant = NULL;
+  GVariant *normal_variant = NULL;
+  GVariant *expected = NULL;
+
+  variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL);
+  g_assert_nonnull (variant);
+
+  g_assert_false (g_variant_is_normal_form (variant));
+
+  normal_variant = g_variant_get_normal_form (variant);
+  g_assert_nonnull (normal_variant);
+  g_assert_cmpuint (g_variant_get_size (normal_variant), <=, size * 3);
+
+  expected = g_variant_new_parsed ("@(ayayay) ([], [], [])");
+  g_assert_cmpvariant (expected, variant);
+  g_assert_cmpvariant (expected, normal_variant);
+
+  g_variant_unref (expected);
+  g_variant_unref (normal_variant);
+  g_variant_unref (variant);
+}
+
 /* Test that an empty object path is normalised successfully to the base object
  * path, ‘/’. */
 static void
@@ -5253,6 +5428,12 @@ main (int argc, char **argv)
                    test_normal_checking_array_offsets2);
   g_test_add_func ("/gvariant/normal-checking/tuple-offsets",
                    test_normal_checking_tuple_offsets);
+  g_test_add_func ("/gvariant/normal-checking/tuple-offsets2",
+                   test_normal_checking_tuple_offsets2);
+  g_test_add_func ("/gvariant/normal-checking/tuple-offsets3",
+                   test_normal_checking_tuple_offsets3);
+  g_test_add_func ("/gvariant/normal-checking/tuple-offsets4",
+                   test_normal_checking_tuple_offsets4);
   g_test_add_func ("/gvariant/normal-checking/empty-object-path",
                    test_normal_checking_empty_object_path);
 
-- 
2.24.4