core/dbus-service: validate type of received ExecContext fds (#39680)

src/basic/fd-util.c

@@ -960,6 +960,41 @@ int fd_is_opath(int fd) {
         return FLAGS_SET(r, O_PATH);
 }
 
+int fd_vet_accmode(int fd, int mode) {
+        int flags;
+
+        /* Check if fd is opened with desired access mode.
+         *
+         * Returns > 0 on strict match, == 0 if opened for both reading and writing (partial match),
+         * -EPROTOTYPE otherwise. O_PATH fds are always refused with -EBADFD.
+         *
+         * Note that while on O_DIRECTORY -EISDIR will be returned, this should not be relied upon as
+         * the flag might not have been specified when open() was called originally. */
+
+        assert(fd >= 0);
+        assert(IN_SET(mode, O_RDONLY, O_WRONLY, O_RDWR));
+
+        flags = fcntl(fd, F_GETFL);
+        if (flags < 0)
+                return -errno;
+
+        if (FLAGS_SET(flags, O_DIRECTORY))
+                return -EISDIR;
+
+        if (FLAGS_SET(flags, O_PATH))
+                return -EBADFD;
+
+        flags &= O_ACCMODE_STRICT;
+
+        if (flags == mode)
+                return 1;
+
+        if (flags == O_RDWR)
+                return 0;
+
+        return -EPROTOTYPE;
+}
+
 int fd_verify_safe_flags_full(int fd, int extra_flags) {
         int flags, unexpected_flags;
 

src/basic/fd-util.h

@@ -151,6 +151,7 @@ int fd_reopen_propagate_append_and_position(int fd, int flags);
 int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd);
 
 int fd_is_opath(int fd);
+int fd_vet_accmode(int fd, int mode);
 
 int fd_verify_safe_flags_full(int fd, int extra_flags);
 static inline int fd_verify_safe_flags(int fd) {

src/core/dbus-service.c

@@ -28,6 +28,7 @@
 #include "selinux-access.h"
 #include "service.h"
 #include "signal-util.h"
+#include "stat-util.h"
 #include "string-util.h"
 #include "unit.h"
 
@@ -488,21 +489,32 @@ static int bus_set_transient_exit_status(
 
 static int bus_set_transient_exec_context_fd(
                 Unit *u,
+                const char *name,
                 int *p,
                 bool *b,
+                int verify_mode,
                 sd_bus_message *message,
                 UnitWriteFlags flags,
                 sd_bus_error *error) {
 
         int fd, r;
 
+        assert(name);
         assert(p);
         assert(b);
+        assert(verify_mode == O_DIRECTORY || (verify_mode & ~O_ACCMODE_STRICT) == 0);
 
         r = sd_bus_message_read(message, "h", &fd);
         if (r < 0)
                 return r;
 
+        if (verify_mode == O_DIRECTORY)
+                r = fd_verify_directory(fd);
+        else
+                r = fd_vet_accmode(fd, verify_mode);
+        if (r < 0)
+                return sd_bus_error_set_errnof(error, r, "%s passed is of incompatible type: %m", name);
+
         if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
                 int copy;
 
@@ -689,13 +701,13 @@ static int bus_service_set_transient_property(
                 return bus_set_transient_exec_command(u, name, &s->exec_command[ci], message, flags, error);
 
         if (streq(name, "StandardInputFileDescriptor"))
-                return bus_set_transient_exec_context_fd(u, &s->stdin_fd, &s->exec_context.stdio_as_fds, message, flags, error);
+                return bus_set_transient_exec_context_fd(u, name, &s->stdin_fd, &s->exec_context.stdio_as_fds, O_RDONLY, message, flags, error);
 
         if (streq(name, "StandardOutputFileDescriptor"))
-                return bus_set_transient_exec_context_fd(u, &s->stdout_fd, &s->exec_context.stdio_as_fds, message, flags, error);
+                return bus_set_transient_exec_context_fd(u, name, &s->stdout_fd, &s->exec_context.stdio_as_fds, O_WRONLY, message, flags, error);
 
         if (streq(name, "StandardErrorFileDescriptor"))
-                return bus_set_transient_exec_context_fd(u, &s->stderr_fd, &s->exec_context.stdio_as_fds, message, flags, error);
+                return bus_set_transient_exec_context_fd(u, name, &s->stderr_fd, &s->exec_context.stdio_as_fds, O_WRONLY, message, flags, error);
 
         if (streq(name, "OpenFile")) {
                 const char *path, *fdname;
@@ -802,7 +814,7 @@ static int bus_service_set_transient_property(
         }
 
         if (streq(name, "RootDirectoryFileDescriptor"))
-                return bus_set_transient_exec_context_fd(u, &s->root_directory_fd, &s->exec_context.root_directory_as_fd, message, flags, error);
+                return bus_set_transient_exec_context_fd(u, name, &s->root_directory_fd, &s->exec_context.root_directory_as_fd, O_DIRECTORY, message, flags, error);
 
         return 0;
 }

src/login/logind-session-dbus.c

@@ -531,8 +531,8 @@ static int method_set_display(sd_bus_message *message, void *userdata, sd_bus_er
 
 static int method_set_tty(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         Session *s = ASSERT_PTR(userdata);
-        int fd, r, flags;
         _cleanup_free_ char *q = NULL;
+        int fd, r;
 
         assert(message);
 
@@ -543,15 +543,11 @@ static int method_set_tty(sd_bus_message *message, void *userdata, sd_bus_error
         if (!session_is_controller(s, sd_bus_message_get_sender(message)))
                 return sd_bus_error_set(error, BUS_ERROR_NOT_IN_CONTROL, "You must be in control of this session to set tty");
 
-        assert(fd >= 0);
-
-        flags = fcntl(fd, F_GETFL, 0);
-        if (flags < 0)
-                return -errno;
-        if ((flags & O_ACCMODE_STRICT) != O_RDWR)
+        r = fd_vet_accmode(fd, O_RDWR);
+        if (r == -EPROTOTYPE)
                 return -EACCES;
-        if (FLAGS_SET(flags, O_PATH))
-                return -ENOTTY;
+        if (r < 0)
+                return r;
 
         r = getttyname_malloc(fd, &q);
         if (r < 0)

src/test/test-fd-util.c

@@ -878,4 +878,29 @@ TEST(fd_get_path) {
         assert_se(chdir(saved_cwd) >= 0);
 }
 
+TEST(fd_vet_accmode) {
+        _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-fd-accmode.XXXXXX";
+        _cleanup_close_ int fd_rw = -EBADF, fd_ro = -EBADF, fd_wo = -EBADF, fd_opath = -EBADF;
+
+        ASSERT_OK(fd_rw = mkostemp_safe(name));
+        ASSERT_OK_ZERO(fd_vet_accmode(fd_rw, O_RDONLY));
+        ASSERT_OK_ZERO(fd_vet_accmode(fd_rw, O_WRONLY));
+        ASSERT_OK_POSITIVE(fd_vet_accmode(fd_rw, O_RDWR));
+
+        ASSERT_OK_ERRNO(fd_ro = open(name, O_RDONLY | O_CLOEXEC));
+        ASSERT_OK_POSITIVE(fd_vet_accmode(fd_ro, O_RDONLY));
+        ASSERT_ERROR(fd_vet_accmode(fd_ro, O_WRONLY), EPROTOTYPE);
+        ASSERT_ERROR(fd_vet_accmode(fd_ro, O_RDWR), EPROTOTYPE);
+
+        ASSERT_OK_ERRNO(fd_wo = open(name, O_WRONLY | O_CLOEXEC));
+        ASSERT_ERROR(fd_vet_accmode(fd_wo, O_RDONLY), EPROTOTYPE);
+        ASSERT_OK_POSITIVE(fd_vet_accmode(fd_wo, O_WRONLY));
+        ASSERT_ERROR(fd_vet_accmode(fd_wo, O_RDWR), EPROTOTYPE);
+
+        ASSERT_OK_ERRNO(fd_opath = open(name, O_PATH | O_CLOEXEC));
+        ASSERT_ERROR(fd_vet_accmode(fd_opath, O_RDONLY), EBADFD);
+        ASSERT_ERROR(fd_vet_accmode(fd_opath, O_WRONLY), EBADFD);
+        ASSERT_ERROR(fd_vet_accmode(fd_opath, O_RDWR), EBADFD);
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);