From 4b611b47e3663fbc43d1cbe7b086196f5408cf34 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Sat, 4 Jan 2014 00:13:52 +0100 Subject: [PATCH] validate incoming names for buses, namespaces, endpoints --- TODO | 2 -- handle.c | 44 ++++++++++++++++++++++++++++++++++++++++---- test/test-kdbus.c | 28 +++++++++++++++++++++------- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/TODO b/TODO index b909966..58c2133 100644 --- a/TODO +++ b/TODO @@ -1,8 +1,6 @@ Features: - also attach queued names to metadata? - - whitelist chars in bus names, they show up in /dev - - account and limit number of messages a connection can have in-flight for another connection, like a connection can have a maximum of 100 messages in-flight, but only 10 of them to the same connection diff --git a/handle.c b/handle.c index a33a1c4..b2da93f 100644 --- a/handle.c +++ b/handle.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "bus.h" #include "connection.h" @@ -202,6 +203,29 @@ static bool kdbus_check_flags(u64 kernel_flags) return kernel_flags <= 0xFFFFFFFFULL; } +static int kdbus_handle_name_valid(const char *name) +{ + unsigned int i; + size_t len; + + len = strlen(name); + if (len == 0) + return -EINVAL; + + for (i = 0; i < len; i++) { + if (isalpha(name[i])) + continue; + if (isdigit(name[i])) + continue; + if (i > 0 && i + 1 < len && strchr("-.", name[i])) + continue; + + return -EINVAL; + } + + return 0; +} + /* kdbus control device commands */ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, void __user *buf) @@ -228,6 +252,10 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, if (ret < 0) break; + ret = kdbus_handle_name_valid(name); + if (ret < 0) + break; + if (!kdbus_check_flags(make->flags)) { ret = -ENOTSUPP; break; @@ -268,6 +296,10 @@ static long kdbus_handle_ioctl_control(struct file *file, unsigned int cmd, if (ret < 0) break; + ret = kdbus_handle_name_valid(name); + if (ret < 0) + break; + if (!kdbus_check_flags(make->flags)) { ret = -ENOTSUPP; break; @@ -321,14 +353,18 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd, case KDBUS_CMD_EP_MAKE: { umode_t mode = 0; kgid_t gid = KGIDT_INIT(0); - char *n; + char *name; if (!KDBUS_IS_ALIGNED8((uintptr_t)buf)) { ret = -EFAULT; break; } - ret = kdbus_ep_make_user(buf, &make, &n); + ret = kdbus_ep_make_user(buf, &make, &name); + if (ret < 0) + break; + + ret = kdbus_handle_name_valid(name); if (ret < 0) break; @@ -344,8 +380,8 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd, gid = current_fsgid(); } - ret = kdbus_ep_new(handle->ep->bus, handle->ep->bus->ns, n, - mode, current_fsuid(), gid, + ret = kdbus_ep_new(handle->ep->bus, handle->ep->bus->ns, + name, mode, current_fsuid(), gid, make->flags & KDBUS_MAKE_POLICY_OPEN); handle->type = KDBUS_HANDLE_EP_OWNER; diff --git a/test/test-kdbus.c b/test/test-kdbus.c index 367f589..42fb29a 100644 --- a/test/test-kdbus.c +++ b/test/test-kdbus.c @@ -333,31 +333,45 @@ static int check_busmake(struct kdbus_check_env *env) bus_make.n_type = KDBUS_ITEM_MAKE_NAME; -#if 0 - /* check some illegal names */ + /* missing uid prefix */ snprintf(bus_make.name, sizeof(bus_make.name), "foo"); bus_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(bus_make.name) + 1; bus_make.head.size = sizeof(struct kdbus_cmd_make) + bus_make.n_size; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == -1 && errno == EINVAL); -#endif + + /* non alphanumeric character */ + snprintf(bus_make.name, sizeof(bus_make.name), "%u-blah@123", getuid()); + bus_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(bus_make.name) + 1; + bus_make.head.size = sizeof(struct kdbus_cmd_make) + + sizeof(uint64_t) * 3 + + bus_make.n_size; + ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); + ASSERT_RETURN(ret == -1 && errno == EINVAL); + + /* '-' at the end */ + snprintf(bus_make.name, sizeof(bus_make.name), "%u-blah-", getuid()); + bus_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(bus_make.name) + 1; + bus_make.head.size = sizeof(struct kdbus_cmd_make) + + sizeof(uint64_t) * 3 + + bus_make.n_size; + ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); + ASSERT_RETURN(ret == -1 && errno == EINVAL); /* create a new bus */ - snprintf(bus_make.name, sizeof(bus_make.name), "%u-blah", getuid()); + snprintf(bus_make.name, sizeof(bus_make.name), "%u-blah-1", getuid()); bus_make.n_size = KDBUS_ITEM_HEADER_SIZE + strlen(bus_make.name) + 1; bus_make.head.size = sizeof(struct kdbus_cmd_make) + sizeof(uint64_t) * 3 + bus_make.n_size; ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == 0); - snprintf(s, sizeof(s), "/dev/" KBUILD_MODNAME "/%u-blah/bus", getuid()); + snprintf(s, sizeof(s), "/dev/" KBUILD_MODNAME "/%u-blah-1/bus", getuid()); ASSERT_RETURN(access(s, F_OK) == 0); -#if 0 /* can't use the same fd for bus make twice */ ret = ioctl(env->control_fd, KDBUS_CMD_BUS_MAKE, &bus_make); ASSERT_RETURN(ret == -1 && errno == EBADFD); -#endif return CHECK_OK; } -- 2.34.1