bus: validate the entire header more closely

3 files changed, 141 insertions, 12 deletions

diff --git a/src/libsystemd-bus/bus-internal.c b/src/libsystemd-bus/bus-internal.c
index e58a8b1da1..d27b3f466b 100644
--- a/src/libsystemd-bus/bus-internal.c
+++ b/src/libsystemd-bus/bus-internal.c
@@ -60,3 +60,111 @@ bool object_path_is_valid(const char *p) {
 
         return true;
 }
+
+bool interface_name_is_valid(const char *p) {
+        const char *q;
+        bool dot, found_dot;
+
+        if (isempty(p))
+                return false;
+
+        for (dot = true, q = p; *q; q++)
+                if (*q == '.') {
+                        if (dot)
+                                return false;
+
+                        found_dot = dot = true;
+                } else {
+                        bool good;
+
+                        good =
+                                (*q >= 'a' && *q <= 'z') ||
+                                (*q >= 'A' && *q <= 'Z') ||
+                                (!dot && *q >= '0' && *q <= '9') ||
+                                *q == '_';
+
+                        if (!good)
+                                return false;
+
+                        dot = false;
+                }
+
+        if (q - p > 255)
+                return false;
+
+        if (dot)
+                return false;
+
+        if (!found_dot)
+                return false;
+
+        return true;
+}
+
+bool service_name_is_valid(const char *p) {
+        const char *q;
+        bool dot, found_dot, unique;
+
+        if (isempty(p))
+                return false;
+
+        unique = p[0] == ':';
+
+        for (dot = true, q = unique ? p+1 : p; *q; q++)
+                if (*q == '.') {
+                        if (dot)
+                                return false;
+
+                        found_dot = dot = true;
+                } else {
+                        bool good;
+
+                        good =
+                                (*q >= 'a' && *q <= 'z') ||
+                                (*q >= 'A' && *q <= 'Z') ||
+                                ((!dot || unique) && *q >= '0' && *q <= '9') ||
+                                *q == '_' || *q == '-';
+
+                        if (!good)
+                                return false;
+
+                        dot = false;
+                }
+
+        if (q - p > 255)
+                return false;
+
+        if (dot)
+                return false;
+
+        if (!found_dot)
+                return false;
+
+        return true;
+
+}
+
+bool member_name_is_valid(const char *p) {
+        const char *q;
+
+        if (isempty(p))
+                return false;
+
+        for (q = p; *q; q++) {
+                bool good;
+
+                good =
+                        (*q >= 'a' && *q <= 'z') ||
+                        (*q >= 'A' && *q <= 'Z') ||
+                        (*q >= '0' && *q <= '9') ||
+                        *q == '_';
+
+                if (!good)
+                        return false;
+        }
+
+        if (q - p > 255)
+                return false;
+
+        return true;
+}
diff --git a/src/libsystemd-bus/bus-internal.h b/src/libsystemd-bus/bus-internal.h
index 809ad823fc..3c2478e8fd 100644
--- a/src/libsystemd-bus/bus-internal.h
+++ b/src/libsystemd-bus/bus-internal.h
@@ -123,3 +123,8 @@ static inline void bus_unrefp(sd_bus **b) {
 #define BUS_ARRAY_MAX_SIZE 67108864
 
 bool object_path_is_valid(const char *p);
+bool interface_name_is_valid(const char *p);
+bool service_name_is_valid(const char *p);
+bool member_name_is_valid(const char *p);
+
+#define error_name_is_valid interface_name_is_valid
diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c
index f5b60f2728..a22962559d 100644
--- a/src/libsystemd-bus/bus-message.c
+++ b/src/libsystemd-bus/bus-message.c
@@ -2123,7 +2123,7 @@ static int message_peek_fields(
 
 static int message_peek_field_string(
                 sd_bus_message *m,
-                char type,
+                bool (*validate)(const char *p),
                 size_t *ri,
                 const char **ret) {
 
@@ -2143,8 +2143,11 @@ static int message_peek_field_string(
         if (r < 0)
                 return r;
 
-        if (type == SD_BUS_TYPE_OBJECT_PATH) {
-                if (!validate_object_path(q, l))
+        if (validate) {
+                if (!validate_nul(q, l))
+                        return -EBADMSG;
+
+                if (!validate(q))
                         return -EBADMSG;
         } else {
                 if (!validate_string(q, l))
@@ -2236,10 +2239,17 @@ static int message_skip_fields(
                 if (!t)
                         return 0;
 
-                if (t == SD_BUS_TYPE_STRING ||
-                    t == SD_BUS_TYPE_OBJECT_PATH) {
+                if (t == SD_BUS_TYPE_STRING) {
+
+                        r = message_peek_field_string(m, NULL, ri, NULL);
+                        if (r < 0)
+                                return r;
+
+                        (*signature)++;
+
+                } else if (t == SD_BUS_TYPE_OBJECT_PATH) {
 
-                        r = message_peek_field_string(m, t, ri, NULL);
+                        r = message_peek_field_string(m, object_path_is_valid, ri, NULL);
                         if (r < 0)
                                 return r;
 
@@ -2366,42 +2376,42 @@ static int message_parse_fields(sd_bus_message *m) {
                         if (!streq(signature, "o"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 'o', &ri, &m->path);
+                        r = message_peek_field_string(m, object_path_is_valid, &ri, &m->path);
                         break;
 
                 case SD_BUS_MESSAGE_HEADER_INTERFACE:
                         if (!streq(signature, "s"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 's', &ri, &m->interface);
+                        r = message_peek_field_string(m, interface_name_is_valid, &ri, &m->interface);
                         break;
 
                 case SD_BUS_MESSAGE_HEADER_MEMBER:
                         if (!streq(signature, "s"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 's', &ri, &m->member);
+                        r = message_peek_field_string(m, member_name_is_valid, &ri, &m->member);
                         break;
 
                 case SD_BUS_MESSAGE_HEADER_ERROR_NAME:
                         if (!streq(signature, "s"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 's', &ri, &m->error.name);
+                        r = message_peek_field_string(m, error_name_is_valid, &ri, &m->error.name);
                         break;
 
                 case SD_BUS_MESSAGE_HEADER_DESTINATION:
                         if (!streq(signature, "s"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 's', &ri, &m->destination);
+                        r = message_peek_field_string(m, service_name_is_valid, &ri, &m->destination);
                         break;
 
                 case SD_BUS_MESSAGE_HEADER_SENDER:
                         if (!streq(signature, "s"))
                                 return -EBADMSG;
 
-                        r = message_peek_field_string(m, 's', &ri, &m->sender);
+                        r = message_peek_field_string(m, service_name_is_valid, &ri, &m->sender);
                         break;
 
 
@@ -2432,6 +2442,12 @@ static int message_parse_fields(sd_bus_message *m) {
                                 return -EBADMSG;
 
                         r = message_peek_field_uint32(m, &ri, &m->reply_serial);
+                        if (r < 0)
+                                return r;
+
+                        if (m->reply_serial == 0)
+                                return -EBADMSG;
+
                         break;
 
                 default: