From efc7d4d1f249873bde58ee68d25c0b7c13538fd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 11 Jan 2015 05:42:15 +0100 Subject: [PATCH] filecopy: prevent files/dirs movement outside incoming directory during transfer Otherwise, when the user moves directory, which is still in transfer, somewhere else, it could allow malicious source domain to escape chroot and place a file in arbitrary location. It looks like bind mount is just enough - simple rename fails with EXDEV, so tools are forced to perform copy+delete, which is enough to keep unpacker process away from new file location. One inconvenient detail is that we must clean the mount after transfer finishes, so root perms cannot be dropped completely. We keep separate process for only that reason. --- qubes-rpc/qfile-unpacker.c | 45 +++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/qubes-rpc/qfile-unpacker.c b/qubes-rpc/qfile-unpacker.c index 7bb8606..a16f2fe 100644 --- a/qubes-rpc/qfile-unpacker.c +++ b/qubes-rpc/qfile-unpacker.c @@ -5,6 +5,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -34,8 +37,11 @@ int prepare_creds_return_uid(const char *username) int main(int argc __attribute((__unused__)), char ** argv __attribute__((__unused__))) { char *incoming_dir; - int uid; + int uid, ret; + pid_t pid; const char *remote_domain; + char *procdir_path; + int procfs_fd; uid = prepare_creds_return_uid("user"); @@ -50,9 +56,36 @@ int main(int argc __attribute((__unused__)), char ** argv __attribute__((__unuse mkdir(incoming_dir, 0700); if (chdir(incoming_dir)) gui_fatal("Error chdir to %s", incoming_dir); - if (chroot(incoming_dir)) //impossible - gui_fatal("Error chroot to %s", incoming_dir); - if (setuid(uid) < 0) - gui_fatal("Error changing permissions to '%s'", "user"); - return do_unpack(); + + if (mount(".", ".", NULL, MS_BIND | MS_NODEV | MS_NOEXEC | MS_NOSUID, NULL) < 0) + gui_fatal("Failed to mount a directory %s", incoming_dir); + + /* parse the input in unprivileged child process, parent will hold root + * access to unmount incoming dir */ + switch (pid=fork()) { + case -1: + gui_fatal("Failed to create new process"); + case 0: + asprintf(&procdir_path, "/proc/%d/fd", getpid()); + procfs_fd = open(procdir_path, O_DIRECTORY | O_RDONLY); + if (procfs_fd < 0) + gui_fatal("Failed to open /proc"); + free(procdir_path); + set_procfs_fd(procfs_fd); + + if (chroot(".")) + gui_fatal("Error chroot to %s", incoming_dir); + if (setuid(uid) < 0) { + /* no kdialog inside chroot */ + perror("setuid"); + exit(1); + } + return do_unpack(); + } + if (waitpid(pid, &ret, 0) < 0) { + gui_fatal("Failed to wait for child process"); + } + if (umount2(".", MNT_DETACH) < 0) + gui_fatal("Cannot umount incoming directory"); + return ret; }