daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
ff6046
From 38a5ae776dc62b42ef5ced8f9812771181af528b Mon Sep 17 00:00:00 2001
ff6046
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
ff6046
Date: Thu, 2 Aug 2018 14:25:11 +0200
ff6046
Subject: [PATCH] bus-message: fix calculation of offsets table
ff6046
ff6046
The offsets specify the ends of variable length data. We would trust the
ff6046
incoming data, putting the offsets specified in our message
ff6046
into the offsets tables after doing some superficial verification.
ff6046
But when actually reading the data we apply alignment, so we would take
ff6046
the previous offset, align it, making it bigger then current offset, and
ff6046
then we'd try to read data of negative length.
ff6046
ff6046
In the attached example, the message specifies the following offsets:
ff6046
[1, 4]
ff6046
but the alignment of those items is
ff6046
[1, 8]
ff6046
so we'd calculate the second item as starting at 8 and ending at 4.
ff6046
ff6046
(cherry picked from commit 12603b84d2fb07603e2ea94b240c6b78ad17510e)
ff6046
ff6046
Resolves: #1696224
ff6046
---
ff6046
 src/libsystemd/sd-bus/bus-message.c           |  36 +++++++++---------
ff6046
 ...h-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 | Bin 0 -> 28 bytes
ff6046
 2 files changed, 18 insertions(+), 18 deletions(-)
ff6046
 create mode 100644 test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5
ff6046
ff6046
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
ff6046
index 81aaa7f59f..d0af34f632 100644
ff6046
--- a/src/libsystemd/sd-bus/bus-message.c
ff6046
+++ b/src/libsystemd/sd-bus/bus-message.c
ff6046
@@ -3140,6 +3140,7 @@ static int container_next_item(sd_bus_message *m, struct bus_container *c, size_
ff6046
                         assert(alignment > 0);
ff6046
 
ff6046
                         *rindex = ALIGN_TO(c->offsets[c->offset_index], alignment);
ff6046
+                        assert(c->offsets[c->offset_index+1] >= *rindex);
ff6046
                         c->item_size = c->offsets[c->offset_index+1] - *rindex;
ff6046
                 } else {
ff6046
 
ff6046
@@ -3179,6 +3180,7 @@ static int container_next_item(sd_bus_message *m, struct bus_container *c, size_
ff6046
                 assert(alignment > 0);
ff6046
 
ff6046
                 *rindex = ALIGN_TO(c->offsets[c->offset_index], alignment);
ff6046
+                assert(c->offsets[c->offset_index+1] >= *rindex);
ff6046
                 c->item_size = c->offsets[c->offset_index+1] - *rindex;
ff6046
 
ff6046
                 c->offset_index++;
ff6046
@@ -3725,7 +3727,7 @@ static int build_struct_offsets(
ff6046
                 size_t *n_offsets) {
ff6046
 
ff6046
         unsigned n_variable = 0, n_total = 0, v;
ff6046
-        size_t previous = 0, where;
ff6046
+        size_t previous, where;
ff6046
         const char *p;
ff6046
         size_t sz;
ff6046
         void *q;
ff6046
@@ -3804,6 +3806,7 @@ static int build_struct_offsets(
ff6046
 
ff6046
         /* Second, loop again and build an offset table */
ff6046
         p = signature;
ff6046
+        previous = m->rindex;
ff6046
         while (*p != 0) {
ff6046
                 size_t n, offset;
ff6046
                 int k;
ff6046
@@ -3817,37 +3820,34 @@ static int build_struct_offsets(
ff6046
                         memcpy(t, p, n);
ff6046
                         t[n] = 0;
ff6046
 
ff6046
+                        size_t align = bus_gvariant_get_alignment(t);
ff6046
+                        assert(align > 0);
ff6046
+
ff6046
+                        /* The possible start of this member after including alignment */
ff6046
+                        size_t start = ALIGN_TO(previous, align);
ff6046
+
ff6046
                         k = bus_gvariant_get_size(t);
ff6046
                         if (k < 0) {
ff6046
                                 size_t x;
ff6046
 
ff6046
-                                /* variable size */
ff6046
+                                /* Variable size */
ff6046
                                 if (v > 0) {
ff6046
                                         v--;
ff6046
 
ff6046
                                         x = bus_gvariant_read_word_le((uint8_t*) q + v*sz, sz);
ff6046
                                         if (x >= size)
ff6046
                                                 return -EBADMSG;
ff6046
-                                        if (m->rindex + x < previous)
ff6046
-                                                return -EBADMSG;
ff6046
                                 } else
ff6046
-                                        /* The last item's end
ff6046
-                                         * is determined from
ff6046
-                                         * the start of the
ff6046
-                                         * offset array */
ff6046
+                                        /* The last item's end is determined
ff6046
+                                         * from the start of the offset array */
ff6046
                                         x = size - (n_variable * sz);
ff6046
 
ff6046
                                 offset = m->rindex + x;
ff6046
-
ff6046
-                        } else {
ff6046
-                                size_t align;
ff6046
-
ff6046
-                                /* fixed size */
ff6046
-                                align = bus_gvariant_get_alignment(t);
ff6046
-                                assert(align > 0);
ff6046
-
ff6046
-                                offset = (*n_offsets == 0 ? m->rindex  : ALIGN_TO((*offsets)[*n_offsets-1], align)) + k;
ff6046
-                        }
ff6046
+                                if (offset < start)
ff6046
+                                        return -EBADMSG;
ff6046
+                        } else
ff6046
+                                /* Fixed size */
ff6046
+                                offset = start + k;
ff6046
                 }
ff6046
 
ff6046
                 previous = (*offsets)[(*n_offsets)++] = offset;
ff6046
diff --git a/test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5 b/test/fuzz/fuzz-bus-message/crash-e1b811da5ca494e494b77c6bd8e1c2f2989425c5
ff6046
new file mode 100644
ff6046
index 0000000000000000000000000000000000000000..9d3fa0035fd360a37833e8b58cc4aea90df9de83
ff6046
GIT binary patch
ff6046
literal 28
ff6046
fcmd1#|DTDG0Z1?a!8`>PAeqj{pplqVrYQgbfcytC
ff6046
ff6046
literal 0
ff6046
HcmV?d00001
ff6046