dbus: add some more safety checks before accepting data from bus clients

5 files changed, 62 insertions, 2 deletions

diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 8f9d5a04f5..cd3ef491ac 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -451,6 +451,14 @@ static DBusHandlerResult hostname_message_handler(
                         } else {
                                 char *h;
 
+                                /* The icon name might ultimately be
+                                 * used as file name, so better be
+                                 * safe than sorry */
+                                if (k == PROP_ICON_NAME && !filename_is_safe(name))
+                                        return bus_send_error_reply(connection, message, NULL, -EINVAL);
+                                if (k == PROP_PRETTY_HOSTNAME && !string_is_safe(name))
+                                        return bus_send_error_reply(connection, message, NULL, -EINVAL);
+
                                 h = strdup(name);
                                 if (!h)
                                         goto oom;
diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index 12fb980dd6..de8d6998cf 100644
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -314,7 +314,7 @@ void server_process_native_file(
                         return;
                 }
 
-                if (strchr(e, '/')) {
+                if (!filename_is_safe(e)) {
                         log_error("Received file in subdirectory of allowed directories. Refusing.");
                         return;
                 }
diff --git a/src/locale/localed.c b/src/locale/localed.c
index a2d381406c..04268a1982 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -1039,7 +1039,9 @@ static DBusHandlerResult locale_message_handler(
                                 size_t k;
 
                                 k = strlen(names[p]);
-                                if (startswith(*i, names[p]) && (*i)[k] == '=') {
+                                if (startswith(*i, names[p]) &&
+                                    (*i)[k] == '=' &&
+                                    string_is_safe((*i) + k + 1)) {
                                         valid = true;
                                         passed[p] = true;
 
@@ -1150,6 +1152,10 @@ static DBusHandlerResult locale_message_handler(
                 if (!streq_ptr(keymap, state.vc_keymap) ||
                     !streq_ptr(keymap_toggle, state.vc_keymap_toggle)) {
 
+                        if ((keymap && (!filename_is_safe(keymap) || !string_is_safe(keymap))) ||
+                            (keymap_toggle && (!filename_is_safe(keymap_toggle) || !string_is_safe(keymap_toggle))))
+                                return bus_send_error_reply(connection, message, NULL, -EINVAL);
+
                         r = verify_polkit(connection, message, "org.freedesktop.locale1.set-keyboard", interactive, NULL, &error);
                         if (r < 0)
                                 return bus_send_error_reply(connection, message, &error, r);
@@ -1220,6 +1226,12 @@ static DBusHandlerResult locale_message_handler(
                     !streq_ptr(variant, state.x11_variant) ||
                     !streq_ptr(options, state.x11_options)) {
 
+                        if ((layout && !string_is_safe(layout)) ||
+                            (model && !string_is_safe(model)) ||
+                            (variant && !string_is_safe(variant)) ||
+                            (options && !string_is_safe(options)))
+                                return bus_send_error_reply(connection, message, NULL, -EINVAL);
+
                         r = verify_polkit(connection, message, "org.freedesktop.locale1.set-keyboard", interactive, NULL, &error);
                         if (r < 0)
                                 return bus_send_error_reply(connection, message, &error, r);
diff --git a/src/shared/util.c b/src/shared/util.c
index d2ca3fc783..64d6e62a53 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -56,6 +56,7 @@
 #include <sys/mman.h>
 #include <sys/vfs.h>
 #include <linux/magic.h>
+#include <limits.h>
 
 #include "macro.h"
 #include "util.h"
@@ -5851,3 +5852,39 @@ void closedirp(DIR **d) {
 void umaskp(mode_t *u) {
         umask(*u);
 }
+
+bool filename_is_safe(const char *p) {
+
+        if (isempty(p))
+                return false;
+
+        if (strchr(p, '/'))
+                return false;
+
+        if (streq(p, "."))
+                return false;
+
+        if (streq(p, ".."))
+                return false;
+
+        if (strlen(p) > FILENAME_MAX)
+                return false;
+
+        return true;
+}
+
+bool string_is_safe(const char *p) {
+        const char *t;
+
+        assert(p);
+
+        for (t = p; *t; t++) {
+                if (*p < ' ')
+                        return false;
+
+                if (strchr("\\\"\'", *p))
+                        return false;
+        }
+
+        return true;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 61b88a8b2e..cbded08617 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -558,3 +558,6 @@ _malloc_ static inline void *memdup_multiply(const void *p, size_t a, size_t b)
 
         return memdup(p, a * b);
 }
+
+bool filename_is_safe(const char *p);
+bool string_is_safe(const char *p);