From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 3 Oct 2012 17:29:20 +0000 (-0400)
Subject: dbus: add some more safety checks before accepting data from bus clients
X-Git-Tag: v194~2
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=0b507b17a760b21e33fc52ff377db6aa5086c680

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

diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 8f9d5a04f..cd3ef491a 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 12fb980dd..de8d6998c 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 a2d381406..04268a198 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 d2ca3fc78..64d6e62a5 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 61b88a8b2..cbded0861 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);