bus-message: avoid an infinite loop on empty structures

The alternative would be to treat gvariant and !gvariant messages differently.
But this is a problem because we check signatures is variuos places before we
have an actual message, for example in sd_bus_add_object_vtable(). It seems
better to treat things consistent (i.e. follow the lowest common denominator)
and disallow empty structures everywhere.

diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index 019c8a5..80d4407 100644 (file)
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -4147,6 +4147,7 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char
 
                         /* signature_element_length does verification internally */
 
+                        /* The array element must not be empty */
                         assert(l >= 1);
                         if (free_and_strndup(&c->peeked_signature,
                                              c->signature + c->index + 1, l) < 0)
@@ -4170,7 +4171,7 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char
                         if (r < 0)
                                 return r;
 
-                        assert(l >= 2);
+                        assert(l >= 3);
                         if (free_and_strndup(&c->peeked_signature,
                                              c->signature + c->index + 1, l - 2) < 0)
                                 return -ENOMEM;

diff --git a/src/libsystemd/sd-bus/bus-signature.c b/src/libsystemd/sd-bus/bus-signature.c
index 06ef1ee..1ecd6e8 100644 (file)
--- a/src/libsystemd/sd-bus/bus-signature.c
+++ b/src/libsystemd/sd-bus/bus-signature.c
@@ -56,6 +56,12 @@ static int signature_element_length_internal(
                         p += t;
                 }
 
+                if (p - s < 2)
+                        /* D-Bus spec: Empty structures are not allowed; there
+                         * must be at least one type code between the parentheses.
+                         */
+                        return -EINVAL;
+
                 *l = p - s + 1;
                 return 0;
         }

diff --git a/src/libsystemd/sd-bus/test-bus-gvariant.c b/src/libsystemd/sd-bus/test-bus-gvariant.c
index 92324bf..8a4ad0e 100644 (file)
--- a/src/libsystemd/sd-bus/test-bus-gvariant.c
+++ b/src/libsystemd/sd-bus/test-bus-gvariant.c
@@ -18,7 +18,7 @@
 
 static void test_bus_gvariant_is_fixed_size(void) {
         assert_se(bus_gvariant_is_fixed_size("") > 0);
-        assert_se(bus_gvariant_is_fixed_size("()") > 0);
+        assert_se(bus_gvariant_is_fixed_size("()") == -EINVAL);
         assert_se(bus_gvariant_is_fixed_size("y") > 0);
         assert_se(bus_gvariant_is_fixed_size("u") > 0);
         assert_se(bus_gvariant_is_fixed_size("b") > 0);
@@ -43,7 +43,7 @@ static void test_bus_gvariant_is_fixed_size(void) {
 
 static void test_bus_gvariant_get_size(void) {
         assert_se(bus_gvariant_get_size("") == 0);
-        assert_se(bus_gvariant_get_size("()") == 1);
+        assert_se(bus_gvariant_get_size("()") == -EINVAL);
         assert_se(bus_gvariant_get_size("y") == 1);
         assert_se(bus_gvariant_get_size("u") == 4);
         assert_se(bus_gvariant_get_size("b") == 1);
@@ -75,7 +75,7 @@ static void test_bus_gvariant_get_size(void) {
 
 static void test_bus_gvariant_get_alignment(void) {
         assert_se(bus_gvariant_get_alignment("") == 1);
-        assert_se(bus_gvariant_get_alignment("()") == 1);
+        assert_se(bus_gvariant_get_alignment("()") == -EINVAL);
         assert_se(bus_gvariant_get_alignment("y") == 1);
         assert_se(bus_gvariant_get_alignment("b") == 1);
         assert_se(bus_gvariant_get_alignment("u") == 4);

diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c
index 9e09b8f..d1c674b 100644 (file)
--- a/src/libsystemd/sd-bus/test-bus-marshal.c
+++ b/src/libsystemd/sd-bus/test-bus-marshal.c
@@ -154,7 +154,7 @@ int main(int argc, char *argv[]) {
         assert_se(r >= 0);
 
         r = sd_bus_message_append(m, "()");
-        assert_se(r >= 0);
+        assert_se(r == -EINVAL);
 
         r = sd_bus_message_append(m, "ba(ss)", 255, 3, "aaa", "1", "bbb", "2", "ccc", "3");
         assert_se(r >= 0);
@@ -296,7 +296,7 @@ int main(int argc, char *argv[]) {
         assert_se(v == 10);
 
         r = sd_bus_message_read(m, "()");
-        assert_se(r > 0);
+        assert_se(r < 0);
 
         r = sd_bus_message_read(m, "ba(ss)", &boolean, 3, &x, &y, &a, &b, &c, &d);
         assert_se(r > 0);
@@ -377,7 +377,7 @@ int main(int argc, char *argv[]) {
 
         assert_se(sd_bus_message_verify_type(m, 'a', "{yv}") > 0);
 
-        r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y()");
+        r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y");
         assert_se(r >= 0);
 
         assert_se(sd_bus_message_verify_type(m, 'b', NULL) > 0);

diff --git a/src/libsystemd/sd-bus/test-bus-signature.c b/src/libsystemd/sd-bus/test-bus-signature.c
index ab59d04..84648db 100644 (file)
--- a/src/libsystemd/sd-bus/test-bus-signature.c
+++ b/src/libsystemd/sd-bus/test-bus-signature.c
@@ -14,9 +14,9 @@ int main(int argc, char *argv[]) {
         assert_se(signature_is_single("v", false));
         assert_se(signature_is_single("as", false));
         assert_se(signature_is_single("(ss)", false));
-        assert_se(signature_is_single("()", false));
-        assert_se(signature_is_single("(()()()()())", false));
-        assert_se(signature_is_single("(((())))", false));
+        assert_se(!signature_is_single("()", false));
+        assert_se(!signature_is_single("(()()()()())", false));
+        assert_se(!signature_is_single("(((())))", false));
         assert_se(signature_is_single("((((s))))", false));
         assert_se(signature_is_single("{ss}", true));
         assert_se(signature_is_single("a{ss}", false));
@@ -61,7 +61,7 @@ int main(int argc, char *argv[]) {
         assert_se(signature_is_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaas", false));
         assert_se(!signature_is_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaau", false));
 
-        assert_se(signature_is_valid("(((((((((((((((((((((((((((((((())))))))))))))))))))))))))))))))", false));
+        assert_se(signature_is_valid("((((((((((((((((((((((((((((((((s))))))))))))))))))))))))))))))))", false));
         assert_se(!signature_is_valid("((((((((((((((((((((((((((((((((()))))))))))))))))))))))))))))))))", false));
 
         assert_se(namespace_complex_pattern("", ""));

diff --git a/test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56 b/test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56
new file mode 100644 (file)
index 0000000..f1bf322
Binary files /dev/null and b/test/fuzz/fuzz-bus-message/crash-26bba7182dedc8848939931d9fcefcb7922f2e56 differ

diff --git a/test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b b/test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b
new file mode 100644 (file)
index 0000000..c975f90
Binary files /dev/null and b/test/fuzz/fuzz-bus-message/timeout-08ee8f6446a4064db064e8e0b3d220147f7d0b5b differ