From a72b0840ca246d6154deca12dec7d854fec3c0da Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sun, 6 Jun 2021 18:49:51 +0200 Subject: selinux enhancements --- src/fcopy/main.c | 17 ++++++++++++----- src/firejail/fs_home.c | 1 + src/firejail/selinux.c | 10 +++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/fcopy/main.c b/src/fcopy/main.c index 869549821..31810de9a 100644 --- a/src/fcopy/main.c +++ b/src/fcopy/main.c @@ -19,11 +19,15 @@ */ #include "../include/common.h" -#include #include #include #include +#include +#ifndef O_PATH +#define O_PATH 010000000 +#endif + #if HAVE_SELINUX #include #include @@ -55,7 +59,7 @@ static void selinux_relabel_path(const char *path, const char *inside_path) { assert(path); assert(inside_path); #if HAVE_SELINUX - char procfs_path[64]; + char procfs_path[64]; char *fcon = NULL; int fd; struct stat st; @@ -69,20 +73,23 @@ static void selinux_relabel_path(const char *path, const char *inside_path) { if (!label_hnd) label_hnd = selabel_open(SELABEL_CTX_FILE, NULL, 0); + if (!label_hnd) + errExit("selabel_open"); + /* Open the file as O_PATH, to pin it while we determine and adjust the label */ - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); + fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); if (fd < 0) return; if (fstat(fd, &st) < 0) goto close; - if (selabel_lookup_raw(label_hnd, &fcon, inside_path, st.st_mode) == 0) { + if (selabel_lookup_raw(label_hnd, &fcon, inside_path, st.st_mode) == 0) { sprintf(procfs_path, "/proc/self/fd/%i", fd); if (arg_debug) printf("Relabeling %s as %s (%s)\n", path, inside_path, fcon); setfilecon_raw(procfs_path, fcon); - } + } freecon(fcon); close: close(fd); diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index 4bcefa443..f61d43c29 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -234,6 +234,7 @@ static void copy_asoundrc(void) { } copy_file_as_user(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR); // regular user + selinux_relabel_path(dest, src); fs_logger2("clone", dest); free(dest); diff --git a/src/firejail/selinux.c b/src/firejail/selinux.c index 06189d7f6..6969e7a3d 100644 --- a/src/firejail/selinux.c +++ b/src/firejail/selinux.c @@ -19,10 +19,13 @@ */ #if HAVE_SELINUX #include "firejail.h" - #include #include + #include +#ifndef O_PATH +#define O_PATH 010000000 +#endif #include #include @@ -52,8 +55,9 @@ void selinux_relabel_path(const char *path, const char *inside_path) if (!label_hnd) errExit("selabel_open"); - /* Open the file as O_PATH, to pin it while we determine and adjust the label */ - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); + /* Open the file as O_PATH, to pin it while we determine and adjust the label + * Defeat symlink races by not allowing symbolic links */ + fd = safer_openat(-1, path, O_NOFOLLOW|O_CLOEXEC|O_PATH); if (fd < 0) return; if (fstat(fd, &st) < 0) -- cgit v1.2.3 From ab9d30c61cb4479aece756a5373e2fe8904a53d7 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sun, 6 Jun 2021 19:49:32 +0200 Subject: blacklist cleaned passwd, group, utmp files just in case users decide to remove them completely from the sandbox, by means of private-etc or whitelist --- src/firejail/fs_var.c | 4 ++++ src/firejail/restrict_users.c | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/firejail/fs_var.c b/src/firejail/fs_var.c index bae3d6df0..20e262d80 100644 --- a/src/firejail/fs_var.c +++ b/src/firejail/fs_var.c @@ -323,4 +323,8 @@ void fs_var_utmp(void) { if (mount(RUN_UTMP_FILE, UTMP_FILE, NULL, MS_BIND|MS_NOSUID|MS_NOEXEC | MS_NODEV | MS_REC, NULL) < 0) errExit("mount bind utmp"); fs_logger2("create", UTMP_FILE); + + // blacklist RUN_UTMP_FILE + if (mount(RUN_RO_FILE, RUN_UTMP_FILE, NULL, MS_BIND, "mode=400,gid=0") < 0) + errExit("mount bind"); } diff --git a/src/firejail/restrict_users.c b/src/firejail/restrict_users.c index 53e395b89..892244b5f 100644 --- a/src/firejail/restrict_users.c +++ b/src/firejail/restrict_users.c @@ -246,6 +246,11 @@ static void sanitize_passwd(void) { // mount-bind tne new password file if (mount(RUN_PASSWD_FILE, "/etc/passwd", "none", MS_BIND, "mode=400,gid=0") < 0) errExit("mount"); + + // blacklist RUN_PASSWD_FILE + if (mount(RUN_RO_FILE, RUN_PASSWD_FILE, "none", MS_BIND, "mode=400,gid=0") < 0) + errExit("mount"); + fs_logger("create /etc/passwd"); return; @@ -376,6 +381,11 @@ static void sanitize_group(void) { // mount-bind tne new group file if (mount(RUN_GROUP_FILE, "/etc/group", "none", MS_BIND, "mode=400,gid=0") < 0) errExit("mount"); + + // blacklist RUN_GROUP_FILE + if (mount(RUN_RO_FILE, RUN_GROUP_FILE, "none", MS_BIND, "mode=400,gid=0") < 0) + errExit("mount"); + fs_logger("create /etc/group"); return; -- cgit v1.2.3 From 35e9a2e30a3b016ffde06b5bedd5e8b1740440b3 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Mon, 7 Jun 2021 11:40:55 +0200 Subject: fix OOB --- src/lib/ldd_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/ldd_utils.c b/src/lib/ldd_utils.c index cd60d74e4..c5dde85b0 100644 --- a/src/lib/ldd_utils.c +++ b/src/lib/ldd_utils.c @@ -50,7 +50,7 @@ int is_lib_64(const char *exe) { unsigned char buf[EI_NIDENT]; ssize_t len = 0; while (len < EI_NIDENT) { - ssize_t sz = read(fd, buf, EI_NIDENT); + ssize_t sz = read(fd, buf + len, EI_NIDENT - len); if (sz <= 0) goto doexit; len += sz; -- cgit v1.2.3 From 4098cb437d822f802cc44e217a1f72f73960b797 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Mon, 7 Jun 2021 22:59:41 +0200 Subject: misc --- src/firejail/dhcp.c | 9 +++------ src/firejail/main.c | 10 ++++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/firejail/dhcp.c b/src/firejail/dhcp.c index 5bcdcad37..47dd39ac0 100644 --- a/src/firejail/dhcp.c +++ b/src/firejail/dhcp.c @@ -153,14 +153,11 @@ void dhcp_start(void) { if (!any_dhcp()) return; - char *dhclient_path = RUN_MNT_DIR "/dhclient";; + char *dhclient_path = RUN_MNT_DIR "/dhclient"; struct stat s; if (stat(dhclient_path, &s) == -1) { - dhclient_path = "/usr/sbin/dhclient"; - if (stat(dhclient_path, &s) == -1) { - fprintf(stderr, "Error: dhclient was not found.\n"); - exit(1); - } + fprintf(stderr, "Error: %s was not found.\n", dhclient_path); + exit(1); } sbox_run(SBOX_ROOT| SBOX_SECCOMP, 4, PATH_FCOPY, "--follow-link", dhclient_path, RUN_MNT_DIR); diff --git a/src/firejail/main.c b/src/firejail/main.c index 7ec2d6114..12ac01de7 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -862,12 +862,11 @@ static void run_cmd_and_exit(int i, int argc, char **argv) { char *guess_shell(void) { const char *shell; char *retval; - struct stat s; shell = env_get("SHELL"); if (shell) { invalid_filename(shell, 0); // no globbing - if (!is_dir(shell) && strstr(shell, "..") == NULL && stat(shell, &s) == 0 && access(shell, X_OK) == 0 && + if (access(shell, X_OK) == 0 && !is_dir(shell) && strstr(shell, "..") == NULL && strcmp(shell, PATH_FIREJAIL) != 0) goto found; } @@ -878,12 +877,15 @@ char *guess_shell(void) { int i = 0; while (shells[i] != NULL) { // access call checks as real UID/GID, not as effective UID/GID - if (stat(shells[i], &s) == 0 && access(shells[i], X_OK) == 0) { + if (access(shells[i], X_OK) == 0) { shell = shells[i]; - break; + goto found; } i++; } + + return NULL; + found: retval = strdup(shell); if (!retval) -- cgit v1.2.3 From 0ad36056e3fe9baec2c7d69444f1eaf1648490a5 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Tue, 8 Jun 2021 12:27:04 +0200 Subject: refactor mounting --- src/firejail/chroot.c | 40 +++-------- src/firejail/dbus.c | 14 +--- src/firejail/firejail.h | 4 ++ src/firejail/fs.c | 163 ++++++++++++++++++++---------------------- src/firejail/fs_home.c | 21 ++---- src/firejail/fs_lib.c | 6 +- src/firejail/fs_trace.c | 6 +- src/firejail/fs_whitelist.c | 18 +---- src/firejail/pulseaudio.c | 6 +- src/firejail/restrict_users.c | 12 +--- src/firejail/util.c | 138 ++++++++++++++++++++++++++--------- src/firejail/x11.c | 25 ++----- 12 files changed, 218 insertions(+), 235 deletions(-) diff --git a/src/firejail/chroot.c b/src/firejail/chroot.c index 757ffb1f7..4125a4130 100644 --- a/src/firejail/chroot.c +++ b/src/firejail/chroot.c @@ -163,12 +163,8 @@ void fs_chroot(const char *rootdir) { int fd = openat(parentfd, "dev", O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) errExit("open"); - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount("/dev", proc, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_path_to_fd("/dev", fd)) errExit("mounting /dev"); - free(proc); close(fd); #ifdef HAVE_X11 @@ -192,11 +188,8 @@ void fs_chroot(const char *rootdir) { fd = openat(parentfd, "tmp/.X11-unix", O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) errExit("open"); - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount("/tmp/.X11-unix", proc, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_path_to_fd("/tmp/.X11-unix", fd)) errExit("mounting /tmp/.X11-unix"); - free(proc); close(fd); } #endif // HAVE_X11 @@ -225,19 +218,11 @@ void fs_chroot(const char *rootdir) { fprintf(stderr, "Error: cannot open %s\n", pulse); exit(1); } - free(pulse); - - char *proc_src, *proc_dst; - if (asprintf(&proc_src, "/proc/self/fd/%d", src) == -1) - errExit("asprintf"); - if (asprintf(&proc_dst, "/proc/self/fd/%d", dst) == -1) - errExit("asprintf"); - if (mount(proc_src, proc_dst, NULL, MS_BIND|MS_REC, NULL) < 0) - errExit("mount bind"); - free(proc_src); - free(proc_dst); + if (bind_mount_by_fd(src, dst)) + errExit("mounting pulseaudio") close(src); close(dst); + free(pulse); // update /etc/machine-id in chroot update_file(parentfd, "etc/machine-id"); @@ -256,11 +241,8 @@ void fs_chroot(const char *rootdir) { fd = openat(parentfd, &RUN_FIREJAIL_LIB_DIR[1], O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) errExit("open"); - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(RUN_FIREJAIL_LIB_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_path_to_fd(RUN_FIREJAIL_LIB_DIR, fd)) errExit("mount bind"); - free(proc); close(fd); // create /run/firejail/mnt directory in chroot @@ -271,11 +253,8 @@ void fs_chroot(const char *rootdir) { fd = openat(parentfd, &RUN_MNT_DIR[1], O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (fd == -1) errExit("open"); - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(RUN_MNT_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_path_to_fd(RUN_MNT_DIR, fd)) errExit("mount bind"); - free(proc); close(fd); // update chroot resolv.conf @@ -289,11 +268,8 @@ void fs_chroot(const char *rootdir) { if (mkdir(oroot, 0755) == -1) errExit("mkdir"); // mount the chroot dir on top of /run/firejail/mnt/oroot in order to reuse the apparmor rules for overlay - if (asprintf(&proc, "/proc/self/fd/%d", parentfd) == -1) - errExit("asprintf"); - if (mount(proc, oroot, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_fd_to_path(parentfd, oroot)) errExit("mounting rootdir oroot"); - free(proc); close(parentfd); // chroot into the new directory if (arg_debug) diff --git a/src/firejail/dbus.c b/src/firejail/dbus.c index b8aa2c974..bfa28fcba 100644 --- a/src/firejail/dbus.c +++ b/src/firejail/dbus.c @@ -258,12 +258,8 @@ static char *find_user_socket_by_format(char *format) { if (asprintf(&dbus_user_socket, format, (int) getuid()) == -1) errExit("asprintf"); struct stat s; - if (stat(dbus_user_socket, &s) == -1) { - if (errno == ENOENT) - goto fail; - return NULL; - errExit("stat"); - } + if (lstat(dbus_user_socket, &s) == -1) + goto fail; if (!S_ISSOCK(s.st_mode)) goto fail; return dbus_user_socket; @@ -426,12 +422,8 @@ static void socket_overlay(char *socket_path, char *proxy_path) { errno = ENOTSOCK; errExit("mounting DBus proxy socket"); } - char *proxy_fd_path; - if (asprintf(&proxy_fd_path, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(proxy_path, socket_path, NULL, MS_BIND | MS_REC, NULL) == -1) + if (bind_mount_fd_to_path(fd, socket_path)) errExit("mount bind"); - free(proxy_fd_path); close(fd); } diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index a5c44739e..ab6e44a8a 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -526,6 +526,10 @@ unsigned extract_timeout(const char *str); void disable_file_or_dir(const char *fname); void disable_file_path(const char *path, const char *file); int safer_openat(int dirfd, const char *path, int flags); +int remount_by_fd(int dst, unsigned long mountflags); +int bind_mount_by_fd(int src, int dst); +int bind_mount_path_to_fd(const char *srcname, int dst); +int bind_mount_fd_to_path(int src, const char *destname); int has_handler(pid_t pid, int signal); void enter_network_namespace(pid_t pid); int read_pid(const char *name, pid_t *pid); diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 09de11de9..eaf23e603 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -54,16 +54,9 @@ static char *opstr[] = { [MOUNT_RDWR_NOCHECK] = "read-write", }; -typedef enum { - UNSUCCESSFUL, - SUCCESSFUL -} LAST_DISABLE_OPERATION; -LAST_DISABLE_OPERATION last_disable = UNSUCCESSFUL; - static void disable_file(OPERATION op, const char *filename) { assert(filename); assert(op dir + len) != '\0' && *(mptr->dir + len) != '/')) && strcmp(path, "/") != 0) // support read-only=/ errLogExit("invalid %s mount", opstr[op]); + fs_logger2(opstr[op], path); - free(proc); - close(fd1); - close(fd2); return; out: @@ -743,8 +748,6 @@ void fs_proc_sys_dev_boot(void) { // disable various ipc sockets in /run/user if (!arg_writable_run_user) { - struct stat s; - char *fname; if (asprintf(&fname, "/run/user/%d", getuid()) == -1) errExit("asprintf"); @@ -755,8 +758,7 @@ void fs_proc_sys_dev_boot(void) { errExit("asprintf"); if (create_empty_dir_as_user(fnamegpg, 0700)) fs_logger2("create", fnamegpg); - if (stat(fnamegpg, &s) == 0) - disable_file(BLACKLIST_FILE, fnamegpg); + disable_file(BLACKLIST_FILE, fnamegpg); free(fnamegpg); // disable /run/user/{uid}/systemd @@ -765,8 +767,7 @@ void fs_proc_sys_dev_boot(void) { errExit("asprintf"); if (create_empty_dir_as_user(fnamesysd, 0755)) fs_logger2("create", fnamesysd); - if (stat(fnamesysd, &s) == 0) - disable_file(BLACKLIST_FILE, fnamesysd); + disable_file(BLACKLIST_FILE, fnamesysd); free(fnamesysd); } free(fname); @@ -781,26 +782,18 @@ void fs_proc_sys_dev_boot(void) { // disable firejail configuration in ~/.config/firejail void disable_config(void) { - struct stat s; - char *fname; if (asprintf(&fname, "%s/.config/firejail", cfg.homedir) == -1) errExit("asprintf"); - if (stat(fname, &s) == 0) - disable_file(BLACKLIST_FILE, fname); + disable_file(BLACKLIST_FILE, fname); free(fname); // disable run time information - if (stat(RUN_FIREJAIL_NETWORK_DIR, &s) == 0) - disable_file(BLACKLIST_FILE, RUN_FIREJAIL_NETWORK_DIR); - if (stat(RUN_FIREJAIL_BANDWIDTH_DIR, &s) == 0) - disable_file(BLACKLIST_FILE, RUN_FIREJAIL_BANDWIDTH_DIR); - if (stat(RUN_FIREJAIL_NAME_DIR, &s) == 0) - disable_file(BLACKLIST_FILE, RUN_FIREJAIL_NAME_DIR); - if (stat(RUN_FIREJAIL_PROFILE_DIR, &s) == 0) - disable_file(BLACKLIST_FILE, RUN_FIREJAIL_PROFILE_DIR); - if (stat(RUN_FIREJAIL_X11_DIR, &s) == 0) - disable_file(BLACKLIST_FILE, RUN_FIREJAIL_X11_DIR); + disable_file(BLACKLIST_FILE, RUN_FIREJAIL_NETWORK_DIR); + disable_file(BLACKLIST_FILE, RUN_FIREJAIL_BANDWIDTH_DIR); + disable_file(BLACKLIST_FILE, RUN_FIREJAIL_NAME_DIR); + disable_file(BLACKLIST_FILE, RUN_FIREJAIL_PROFILE_DIR); + disable_file(BLACKLIST_FILE, RUN_FIREJAIL_X11_DIR); } @@ -1241,8 +1234,8 @@ void fs_private_tmp(void) { // whitelist x11 directory profile_add("whitelist /tmp/.X11-unix"); - // read-only x11 directory - profile_add("read-only /tmp/.X11-unix"); + // read-only x11 directory + profile_add("read-only /tmp/.X11-unix"); // whitelist any pulse* file in /tmp directory // some distros use PulseAudio sockets under /tmp instead of the socket in /urn/user diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index f61d43c29..ec6dab947 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -287,17 +287,9 @@ void fs_private_homedir(void) { exit(1); } // mount via the links in /proc/self/fd - char *proc_src, *proc_dst; - if (asprintf(&proc_src, "/proc/self/fd/%d", src) == -1) - errExit("asprintf"); - if (asprintf(&proc_dst, "/proc/self/fd/%d", dst) == -1) - errExit("asprintf"); - if (mount(proc_src, proc_dst, NULL, MS_NOSUID | MS_NODEV | MS_BIND | MS_REC, NULL) < 0) + if (bind_mount_by_fd(src, dst)) errExit("mount bind"); - free(proc_src); - free(proc_dst); - close(src); - close(dst); + // check /proc/self/mountinfo to confirm the mount is ok MountData *mptr = get_last_mount(); size_t len = strlen(homedir); @@ -305,6 +297,8 @@ void fs_private_homedir(void) { (*(mptr->dir + len) != '\0' && *(mptr->dir + len) != '/')) errLogExit("invalid private mount"); + close(src); + close(dst); fs_logger3("mount-bind", private_homedir, homedir); fs_logger2("whitelist", homedir); // preserve mode and ownership @@ -590,13 +584,10 @@ void fs_private_home_list(void) { exit(1); } // mount using the file descriptor - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(RUN_HOME_DIR, proc, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_path_to_fd(RUN_HOME_DIR, fd)) errExit("mount bind"); - free(proc); close(fd); + // check /proc/self/mountinfo to confirm the mount is ok MountData *mptr = get_last_mount(); if (strcmp(mptr->dir, homedir) != 0 || strcmp(mptr->fstype, "tmpfs") != 0) diff --git a/src/firejail/fs_lib.c b/src/firejail/fs_lib.c index 5df356d04..8369e6259 100644 --- a/src/firejail/fs_lib.c +++ b/src/firejail/fs_lib.c @@ -203,7 +203,7 @@ void fslib_mount_libs(const char *full_path, unsigned user) { } if (arg_debug || arg_debug_private_lib) - printf(" fslib_mount_libs %s (parse as %s)\n", full_path, user ? "user" : "root"); + printf(" fslib_mount_libs %s\n", full_path); // create an empty RUN_LIB_FILE and allow the user to write to it unlink(RUN_LIB_FILE); // in case is there create_empty_file_as_root(RUN_LIB_FILE, 0644); @@ -212,7 +212,7 @@ void fslib_mount_libs(const char *full_path, unsigned user) { // run fldd to extract the list of files if (arg_debug || arg_debug_private_lib) - printf(" running fldd %s\n", full_path); + printf(" running fldd %s as %s\n", full_path, user ? "user" : "root"); unsigned mask; if (user) mask = SBOX_USER; @@ -298,7 +298,7 @@ static void install_list_entry(const char *lib) { //printf("glob %s\n", globbuf.gl_pathv[j]); // GLOB_NOCHECK - no pattern matched returns the original pattern; try to load it anyway - // foobar/* includes foobar/. and foobar/.. + // foobar/* expands to foobar/. and foobar/.. const char *base = gnu_basename(globbuf.gl_pathv[j]); if (strcmp(base, ".") == 0 || strcmp(base, "..") == 0) continue; diff --git a/src/firejail/fs_trace.c b/src/firejail/fs_trace.c index 1fc38361e..475a391ec 100644 --- a/src/firejail/fs_trace.c +++ b/src/firejail/fs_trace.c @@ -71,12 +71,8 @@ void fs_tracefile(void) { // mount using the symbolic link in /proc/self/fd if (arg_debug) printf("Bind mount %s to %s\n", arg_tracefile, RUN_TRACE_FILE); - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(proc, RUN_TRACE_FILE, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_fd_to_path(fd, RUN_TRACE_FILE)) errExit("mount bind " RUN_TRACE_FILE); - free(proc); close(fd); // now that RUN_TRACE_FILE is user-writable, mount it noexec fs_remount(RUN_TRACE_FILE, MOUNT_NOEXEC, 0); diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c index 258f023f6..ec3fe4c5b 100644 --- a/src/firejail/fs_whitelist.c +++ b/src/firejail/fs_whitelist.c @@ -195,15 +195,7 @@ static void whitelist_file(int dirfd, const char *relpath, const char *path) { if (arg_debug || arg_debug_whitelists) printf("Whitelisting %s\n", path); - - // in order to make this mount resilient against symlink attacks, use - // magic links in /proc/self/fd instead of mounting the paths directly - char *proc_src, *proc_dst; - if (asprintf(&proc_src, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (asprintf(&proc_dst, "/proc/self/fd/%d", fd3) == -1) - errExit("asprintf"); - if (mount(proc_src, proc_dst, NULL, MS_BIND | MS_REC, NULL) < 0) + if (bind_mount_by_fd(fd, fd3)) errExit("mount bind"); // check the last mount operation MountData *mptr = get_last_mount(); // will do exit(1) if the mount cannot be found @@ -221,8 +213,6 @@ static void whitelist_file(int dirfd, const char *relpath, const char *path) { // - there should be more than one '/' char in dest string if (mptr->dir == strrchr(mptr->dir, '/')) errLogExit("invalid whitelist mount"); - free(proc_src); - free(proc_dst); close(fd); close(fd3); fs_logger2("whitelist", path); @@ -341,12 +331,8 @@ static void tmpfs_topdirs(const TopDir *topdirs) { // restore /run/firejail directory if (mkdir(RUN_FIREJAIL_DIR, 0755) == -1) errExit("mkdir"); - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(proc, RUN_FIREJAIL_DIR, NULL, MS_BIND | MS_REC, NULL) < 0) + if (bind_mount_fd_to_path(fd, RUN_FIREJAIL_DIR)) errExit("mount bind"); - free(proc); close(fd); fs_logger2("whitelist", RUN_FIREJAIL_DIR); diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index 1b01a71c6..be0f5aea6 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -158,17 +158,13 @@ void pulseaudio_init(void) { // mount via the link in /proc/self/fd if (arg_debug) printf("Mounting %s on %s\n", RUN_PULSE_DIR, homeusercfg); - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(RUN_PULSE_DIR, proc, "none", MS_BIND, NULL) < 0) + if (bind_mount_path_to_fd(RUN_PULSE_DIR, fd)) errExit("mount pulseaudio"); // check /proc/self/mountinfo to confirm the mount is ok MountData *mptr = get_last_mount(); if (strcmp(mptr->dir, homeusercfg) != 0 || strcmp(mptr->fstype, "tmpfs") != 0) errLogExit("invalid pulseaudio mount"); fs_logger2("tmpfs", homeusercfg); - free(proc); close(fd); char *p; diff --git a/src/firejail/restrict_users.c b/src/firejail/restrict_users.c index 892244b5f..6f17231a4 100644 --- a/src/firejail/restrict_users.c +++ b/src/firejail/restrict_users.c @@ -104,12 +104,8 @@ static void sanitize_home(void) { selinux_relabel_path(cfg.homedir, cfg.homedir); // bring back real user home directory - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(proc, cfg.homedir, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_fd_to_path(fd, cfg.homedir)) errExit("mount bind"); - free(proc); close(fd); if (!arg_private) @@ -154,12 +150,8 @@ static void sanitize_run(void) { selinux_relabel_path(runuser, runuser); // bring back real run/user/$UID directory - char *proc; - if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) - errExit("asprintf"); - if (mount(proc, runuser, NULL, MS_BIND|MS_REC, NULL) < 0) + if (bind_mount_fd_to_path(fd, runuser)) errExit("mount bind"); - free(proc); close(fd); fs_logger2("whitelist", runuser); diff --git a/src/firejail/util.c b/src/firejail/util.c index dc9fe2449..44f1cbd02 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -394,11 +394,11 @@ void touch_file_as_user(const char *fname, mode_t mode) { // drop privileges drop_privs(0); - FILE *fp = fopen(fname, "wx"); - if (fp) { - fprintf(fp, "\n"); - SET_PERMS_STREAM(fp, -1, -1, mode); - fclose(fp); + int fd = open(fname, O_RDONLY|O_CREAT|O_EXCL|O_CLOEXEC, S_IRUSR | S_IWUSR); + if (fd > -1) { + int err = fchmod(fd, mode); + (void) err; + close(fd); } else fwarning("cannot create %s\n", fname); @@ -899,27 +899,29 @@ int remove_overlay_directory(void) { errExit("asprintf"); if (lstat(path, &s) == 0) { - // deal with obvious problems such as symlinks and root ownership - if (!S_ISDIR(s.st_mode)) { - if (S_ISLNK(s.st_mode)) - fprintf(stderr, "Error: %s is a symbolic link\n", path); - else - fprintf(stderr, "Error: %s is not a directory\n", path); - exit(1); - } - if (s.st_uid != getuid()) { - fprintf(stderr, "Error: %s is not owned by the current user\n", path); - exit(1); - } - pid_t child = fork(); if (child < 0) errExit("fork"); if (child == 0) { - // open ~/.firejail, fails if there is any symlink - int fd = safer_openat(-1, path, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); - if (fd == -1) - errExit("safer_openat"); + // open ~/.firejail + int fd = safer_openat(-1, path, O_PATH|O_NOFOLLOW|O_CLOEXEC); + if (fd == -1) { + fprintf(stderr, "Error: cannot open %s\n", path); + _exit(1); + } + if (fstat(fd, &s) == -1) + errExit("fstat"); + if (!S_ISDIR(s.st_mode)) { + if (S_ISLNK(s.st_mode)) + fprintf(stderr, "Error: %s is a symbolic link\n", path); + else + fprintf(stderr, "Error: %s is not a directory\n", path); + _exit(1); + } + if (s.st_uid != getuid()) { + fprintf(stderr, "Error: %s is not owned by the current user\n", path); + _exit(1); + } // chdir to ~/.firejail if (fchdir(fd) == -1) errExit("fchdir"); @@ -988,8 +990,8 @@ int create_empty_dir_as_user(const char *dir, mode_t mode) { drop_privs(0); if (mkdir(dir, mode) == 0) { - if (chmod(dir, mode) == -1) - {;} // do nothing + int err = chmod(dir, mode); + (void) err; } else if (arg_debug) printf("Directory %s not created: %s\n", dir, strerror(errno)); @@ -1110,20 +1112,32 @@ unsigned extract_timeout(const char *str) { } void disable_file_or_dir(const char *fname) { + assert(fname); + + int fd = open(fname, O_PATH|O_CLOEXEC); + if (fd < 0) + return; + struct stat s; - if (stat(fname, &s) != -1) { - if (arg_debug) - printf("blacklist %s\n", fname); - if (is_dir(fname)) { - if (mount(RUN_RO_DIR, fname, "none", MS_BIND, "mode=400,gid=0") < 0) + if (fstat(fd, &s) < 0) { // FUSE + if (errno != EACCES) + errExit("fstat"); + close(fd); + return; + } + + if (arg_debug) + printf("blacklist %s\n", fname); + if (S_ISDIR(s.st_mode)) { + if (bind_mount_path_to_fd(RUN_RO_DIR, fd) < 0) errExit("disable directory"); - } - else { - if (mount(RUN_RO_FILE, fname, "none", MS_BIND, "mode=400,gid=0") < 0) - errExit("disable file"); - } - fs_logger2("blacklist", fname); } + else { + if (bind_mount_path_to_fd(RUN_RO_FILE, fd) < 0) + errExit("disable file"); + } + close(fd); + fs_logger2("blacklist", fname); } void disable_file_path(const char *path, const char *file) { @@ -1210,6 +1224,60 @@ int safer_openat(int dirfd, const char *path, int flags) { return fd; } +int remount_by_fd(int dst, unsigned long mountflags) { + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", dst) < 0) + errExit("asprintf"); + + int rv = mount(NULL, proc, NULL, mountflags|MS_BIND|MS_REMOUNT, NULL); + if (rv < 0 && arg_debug) + printf("Failed mount: %s\n", strerror(errno)); + + free(proc); + return rv; +} + +int bind_mount_by_fd(int src, int dst) { + char *proc_src, *proc_dst; + if (asprintf(&proc_src, "/proc/self/fd/%d", src) < 0 || + asprintf(&proc_dst, "/proc/self/fd/%d", dst) < 0) + errExit("asprintf"); + + int rv = mount(proc_src, proc_dst, NULL, MS_BIND|MS_REC, NULL); + if (rv < 0 && arg_debug) + printf("Failed mount: %s\n", strerror(errno)); + + free(proc_src); + free(proc_dst); + return rv; +} + +int bind_mount_fd_to_path(int src, const char *destname) { + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", src) < 0) + errExit("asprintf"); + + int rv = mount(proc, destname, NULL, MS_BIND|MS_REC, NULL); + if (rv < 0 && arg_debug) + printf("Failed mount: %s\n", strerror(errno)); + + free(proc); + return rv; +} + +int bind_mount_path_to_fd(const char *srcname, int dst) { + char *proc; + if (asprintf(&proc, "/proc/self/fd/%d", dst) < 0) + errExit("asprintf"); + + int rv = mount(srcname, proc, NULL, MS_BIND|MS_REC, NULL); + if (rv < 0 && arg_debug) + printf("Failed mount: %s\n", strerror(errno)); + + free(proc); + return rv; +} + int has_handler(pid_t pid, int signal) { if (signal > 0 && signal <= SIGRTMAX) { char *fname; diff --git a/src/firejail/x11.c b/src/firejail/x11.c index f4f093138..afe77e246 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -1276,12 +1276,7 @@ void x11_xorg(void) { // mount via the link in /proc/self/fd if (arg_debug) printf("Mounting %s on %s\n", tmpfname, dest); - char *proc_src, *proc_dst; - if (asprintf(&proc_src, "/proc/self/fd/%d", src) == -1) - errExit("asprintf"); - if (asprintf(&proc_dst, "/proc/self/fd/%d", dst) == -1) - errExit("asprintf"); - if (mount(proc_src, proc_dst, NULL, MS_BIND, NULL) == -1) { + if (bind_mount_by_fd(src, dst)) { fprintf(stderr, "Error: cannot mount the new .Xauthority file\n"); exit(1); } @@ -1289,8 +1284,6 @@ void x11_xorg(void) { MountData *mptr = get_last_mount(); if (strcmp(mptr->dir, dest) != 0 || strcmp(mptr->fstype, "tmpfs") != 0) errLogExit("invalid .Xauthority mount"); - free(proc_src); - free(proc_dst); close(src); close(dst); @@ -1336,6 +1329,8 @@ void fs_x11(void) { return; } + // the mount source is under control of the user, so be careful and + // mount without following symbolic links, using a file descriptor char *x11file; if (asprintf(&x11file, "/tmp/.X11-unix/X%d", display) == -1) errExit("asprintf"); @@ -1344,10 +1339,10 @@ void fs_x11(void) { free(x11file); return; } - struct stat x11stat; - if (fstat(src, &x11stat) < 0) + struct stat s3; + if (fstat(src, &s3) < 0) errExit("fstat"); - if (!S_ISSOCK(x11stat.st_mode)) { + if (!S_ISSOCK(s3.st_mode)) { close(src); free(x11file); return; @@ -1367,14 +1362,8 @@ void fs_x11(void) { if (dst < 0) errExit("open"); - char *proc_src, *proc_dst; - if (asprintf(&proc_src, "/proc/self/fd/%d", src) == -1 || - asprintf(&proc_dst, "/proc/self/fd/%d", dst) == -1) - errExit("asprintf"); - if (mount(proc_src, proc_dst, NULL, MS_BIND | MS_REC, NULL) < 0) + if (bind_mount_by_fd(src, dst)) errExit("mount bind"); - free(proc_src); - free(proc_dst); close(src); close(dst); fs_logger2("whitelist", x11file); -- cgit v1.2.3 From 5b921120a3615534b8cfce39090224c1e22edb47 Mon Sep 17 00:00:00 2001 From: smitsohu Date: Tue, 8 Jun 2021 16:00:19 +0200 Subject: add more EUID switching always access files under control of the user with effective user id of the user --- src/firejail/checkcfg.c | 3 +- src/firejail/chroot.c | 2 +- src/firejail/firejail.h | 3 ++ src/firejail/fs.c | 47 ++++++++++++++++++------ src/firejail/fs_dev.c | 2 + src/firejail/fs_home.c | 41 +++++++++++---------- src/firejail/fs_lib.c | 7 ++-- src/firejail/fs_whitelist.c | 1 + src/firejail/macros.c | 1 + src/firejail/mountinfo.c | 5 ++- src/firejail/paths.c | 2 +- src/firejail/sbox.c | 2 +- src/firejail/util.c | 89 ++++++++++++++++++++++++++++++++++++++++++--- src/firejail/x11.c | 15 +++----- 14 files changed, 165 insertions(+), 55 deletions(-) diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c index d7690a4fc..f3ab0a6d8 100644 --- a/src/firejail/checkcfg.c +++ b/src/firejail/checkcfg.c @@ -134,8 +134,7 @@ int checkcfg(int val) { *end = '\0'; // is the file present? - struct stat s; - if (stat(fname, &s) == -1) { + if (access(fname, F_OK) == -1) { fprintf(stderr, "Error: netfilter-default file %s not available\n", fname); exit(1); } diff --git a/src/firejail/chroot.c b/src/firejail/chroot.c index 4125a4130..05d89a866 100644 --- a/src/firejail/chroot.c +++ b/src/firejail/chroot.c @@ -219,7 +219,7 @@ void fs_chroot(const char *rootdir) { exit(1); } if (bind_mount_by_fd(src, dst)) - errExit("mounting pulseaudio") + errExit("mounting pulseaudio"); close(src); close(dst); free(pulse); diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index ab6e44a8a..10133142a 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -503,6 +503,9 @@ void copy_file_from_user_to_root(const char *srcname, const char *destname, uid_ void touch_file_as_user(const char *fname, mode_t mode); int is_dir(const char *fname); int is_link(const char *fname); +char *realpath_as_user(const char *fname); +int stat_as_user(const char *fname, struct stat *s); +int lstat_as_user(const char *fname, struct stat *s); void trim_trailing_slash_or_dot(char *path); char *line_remove_spaces(const char *buf); char *split_comma(char *str); diff --git a/src/firejail/fs.c b/src/firejail/fs.c index eaf23e603..01182bd2c 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -57,6 +57,7 @@ static char *opstr[] = { static void disable_file(OPERATION op, const char *filename) { assert(filename); assert(op next; continue; @@ -376,16 +387,12 @@ void fs_blacklist(void) { op = MOUNT_TMPFS; } else if (strncmp(entry->data, "mkdir ", 6) == 0) { - EUID_USER(); fs_mkdir(entry->data + 6); - EUID_ROOT(); entry = entry->next; continue; } else if (strncmp(entry->data, "mkfile ", 7) == 0) { - EUID_USER(); fs_mkfile(entry->data + 7); - EUID_ROOT(); entry = entry->next; continue; } @@ -441,6 +448,8 @@ void fs_blacklist(void) { for (i = 0; i < noblacklist_c; i++) free(noblacklist[i]); free(noblacklist); + + EUID_ROOT(); } //*********************************************** @@ -449,6 +458,7 @@ void fs_blacklist(void) { // mount a writable tmpfs on directory; requires a resolved path void fs_tmpfs(const char *dir, unsigned check_owner) { + EUID_USER(); assert(dir); if (arg_debug) printf("Mounting tmpfs on %s, check owner: %s\n", dir, (check_owner)? "yes": "no"); @@ -473,6 +483,7 @@ void fs_tmpfs(const char *dir, unsigned check_owner) { errExit("fstatvfs"); unsigned long flags = buf.f_flag & ~(MS_RDONLY|MS_BIND); // mount via the symbolic link in /proc/self/fd + EUID_ROOT(); char *proc; if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1) errExit("asprintf"); @@ -490,6 +501,7 @@ void fs_tmpfs(const char *dir, unsigned check_owner) { // remount path, preserving other mount flags; requires a resolved path static void fs_remount_simple(const char *path, OPERATION op) { + EUID_ASSERT(); assert(path); // open path without following symbolic links @@ -555,7 +567,9 @@ static void fs_remount_simple(const char *path, OPERATION op) { // make path a mount point: // mount --bind path path + EUID_ROOT(); int err = bind_mount_by_fd(fd, fd); + EUID_USER(); if (err) { close(fd); goto out; @@ -575,7 +589,9 @@ static void fs_remount_simple(const char *path, OPERATION op) { if (s.st_dev != s2.st_dev || s.st_ino != s2.st_ino) errLogExit("invalid %s mount", opstr[op]); + EUID_ROOT(); err = remount_by_fd(fd2, flags); + EUID_USER(); close(fd2); if (err) goto out; @@ -599,7 +615,9 @@ out: // remount recursively; requires a resolved path static void fs_remount_rec(const char *dir, OPERATION op) { + EUID_ASSERT(); assert(dir); + struct stat s; if (stat(dir, &s) != 0) return; @@ -637,6 +655,9 @@ static void fs_remount_rec(const char *dir, OPERATION op) { // resolve a path and remount it void fs_remount(const char *path, OPERATION op, int rec) { assert(path); + assert(geteuid() == 0); + EUID_USER(); + char *rpath = realpath(path, NULL); if (rpath) { if (rec) @@ -645,10 +666,12 @@ void fs_remount(const char *path, OPERATION op, int rec) { fs_remount_simple(rpath, op); free(rpath); } + EUID_ROOT(); } // Disable /mnt, /media, /run/mount and /run/media access void fs_mnt(const int enforce) { + EUID_USER(); if (enforce) { // disable-mnt set in firejail.config // overriding with noblacklist is not possible in this case @@ -658,13 +681,12 @@ void fs_mnt(const int enforce) { disable_file(BLACKLIST_FILE, "/run/media"); } else { - EUID_USER(); profile_add("blacklist /mnt"); profile_add("blacklist /media"); profile_add("blacklist /run/mount"); profile_add("blacklist /run/media"); - EUID_ROOT(); } + EUID_ROOT(); } @@ -679,7 +701,6 @@ void fs_proc_sys_dev_boot(void) { errExit("mounting /proc/sys"); fs_logger("read-only /proc/sys"); - /* Mount a version of /sys that describes the network namespace */ if (arg_debug) printf("Remounting /sys directory\n"); @@ -694,13 +715,13 @@ void fs_proc_sys_dev_boot(void) { else fs_logger("remount /sys"); + EUID_USER(); + disable_file(BLACKLIST_FILE, "/sys/firmware"); disable_file(BLACKLIST_FILE, "/sys/hypervisor"); { // allow user access to some directories in /sys/ by specifying 'noblacklist' option - EUID_USER(); profile_add("blacklist /sys/fs"); profile_add("blacklist /sys/module"); - EUID_ROOT(); } disable_file(BLACKLIST_FILE, "/sys/power"); disable_file(BLACKLIST_FILE, "/sys/kernel/debug"); @@ -744,8 +765,6 @@ void fs_proc_sys_dev_boot(void) { // disable /dev/port disable_file(BLACKLIST_FILE, "/dev/port"); - - // disable various ipc sockets in /run/user if (!arg_writable_run_user) { char *fname; @@ -778,10 +797,13 @@ void fs_proc_sys_dev_boot(void) { disable_file(BLACKLIST_FILE, "/dev/kmsg"); disable_file(BLACKLIST_FILE, "/proc/kmsg"); } + + EUID_ROOT(); } // disable firejail configuration in ~/.config/firejail void disable_config(void) { + EUID_USER(); char *fname; if (asprintf(&fname, "%s/.config/firejail", cfg.homedir) == -1) errExit("asprintf"); @@ -794,6 +816,7 @@ void disable_config(void) { disable_file(BLACKLIST_FILE, RUN_FIREJAIL_NAME_DIR); disable_file(BLACKLIST_FILE, RUN_FIREJAIL_PROFILE_DIR); disable_file(BLACKLIST_FILE, RUN_FIREJAIL_X11_DIR); + EUID_ROOT(); } @@ -855,6 +878,7 @@ void fs_basic_fs(void) { #ifdef HAVE_OVERLAYFS char *fs_check_overlay_dir(const char *subdirname, int allow_reuse) { assert(subdirname); + EUID_ASSERT(); struct stat s; char *dirname; @@ -1214,6 +1238,7 @@ void fs_overlayfs(void) { // this function is called from sandbox.c before blacklist/whitelist functions void fs_private_tmp(void) { + EUID_ASSERT(); if (arg_debug) printf("Generate private-tmp whitelist commands\n"); diff --git a/src/firejail/fs_dev.c b/src/firejail/fs_dev.c index 8c2870a4d..8cc3ecc62 100644 --- a/src/firejail/fs_dev.c +++ b/src/firejail/fs_dev.c @@ -187,8 +187,10 @@ static void mount_dev_shm(void) { static void process_dev_shm(void) { // Jack audio keeps an Unix socket under (/dev/shm/jack_default_1000_0 or /dev/shm/jack/...) // looking for jack socket + EUID_USER(); glob_t globbuf; int globerr = glob(RUN_DEV_DIR "/shm/jack*", GLOB_NOSORT, NULL, &globbuf); + EUID_ROOT(); if (globerr && !arg_keep_dev_shm) { empty_dev_shm(); return; diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index ec6dab947..eab952eb8 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -42,15 +42,14 @@ static void skel(const char *homedir, uid_t u, gid_t g) { // copy skel files if (asprintf(&fname, "%s/.zshrc", homedir) == -1) errExit("asprintf"); - struct stat s; // don't copy it if we already have the file - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) return; - if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + if (is_link(fname)) { // access(3) on dangling symlinks fails, try again using lstat fprintf(stderr, "Error: invalid %s file\n", fname); exit(1); } - if (stat("/etc/skel/.zshrc", &s) == 0) { + if (access("/etc/skel/.zshrc", R_OK) == 0) { copy_file_as_user("/etc/skel/.zshrc", fname, u, g, 0644); // regular user fs_logger("clone /etc/skel/.zshrc"); fs_logger2("clone", fname); @@ -67,16 +66,14 @@ static void skel(const char *homedir, uid_t u, gid_t g) { // copy skel files if (asprintf(&fname, "%s/.cshrc", homedir) == -1) errExit("asprintf"); - struct stat s; - // don't copy it if we already have the file - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) return; - if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + if (is_link(fname)) { // access(3) on dangling symlinks fails, try again using lstat fprintf(stderr, "Error: invalid %s file\n", fname); exit(1); } - if (stat("/etc/skel/.cshrc", &s) == 0) { + if (access("/etc/skel/.cshrc", R_OK) == 0) { copy_file_as_user("/etc/skel/.cshrc", fname, u, g, 0644); // regular user fs_logger("clone /etc/skel/.cshrc"); fs_logger2("clone", fname); @@ -93,15 +90,14 @@ static void skel(const char *homedir, uid_t u, gid_t g) { // copy skel files if (asprintf(&fname, "%s/.bashrc", homedir) == -1) errExit("asprintf"); - struct stat s; // don't copy it if we already have the file - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) return; - if (is_link(fname)) { // stat on dangling symlinks fails, try again using lstat + if (is_link(fname)) { // access(3) on dangling symlinks fails, try again using lstat fprintf(stderr, "Error: invalid %s file\n", fname); exit(1); } - if (stat("/etc/skel/.bashrc", &s) == 0) { + if (access("/etc/skel/.bashrc", R_OK) == 0) { copy_file_as_user("/etc/skel/.bashrc", fname, u, g, 0644); // regular user fs_logger("clone /etc/skel/.bashrc"); fs_logger2("clone", fname); @@ -122,8 +118,8 @@ static int store_xauthority(void) { errExit("asprintf"); struct stat s; - if (stat(src, &s) == 0) { - if (is_link(src)) { + if (lstat_as_user(src, &s) == 0) { + if (S_ISLNK(s.st_mode)) { fwarning("invalid .Xauthority file\n"); free(src); return 0; @@ -161,11 +157,11 @@ static int store_asoundrc(void) { errExit("asprintf"); struct stat s; - if (stat(src, &s) == 0) { - if (is_link(src)) { + if (lstat_as_user(src, &s) == 0) { + if (S_ISLNK(s.st_mode)) { // make sure the real path of the file is inside the home directory /* coverity[toctou] */ - char* rp = realpath(src, NULL); + char *rp = realpath_as_user(src); if (!rp) { fprintf(stderr, "Error: Cannot access %s\n", src); exit(1); @@ -263,6 +259,7 @@ void fs_private_homedir(void) { if (arg_debug) printf("Mount-bind %s on top of %s\n", private_homedir, homedir); // get file descriptors for homedir and private_homedir, fails if there is any symlink + EUID_USER(); int src = safer_openat(-1, private_homedir, O_PATH|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC); if (src == -1) errExit("opening private directory"); @@ -287,6 +284,7 @@ void fs_private_homedir(void) { exit(1); } // mount via the links in /proc/self/fd + EUID_ROOT(); if (bind_mount_by_fd(src, dst)) errExit("mount bind"); @@ -433,6 +431,7 @@ void fs_check_private_cwd(const char *dir) { // --private-home //*********************************************************************************** static char *check_dir_or_file(const char *name) { + EUID_ASSERT(); assert(name); // basic checks @@ -493,6 +492,7 @@ errexit: } static void duplicate(char *name) { + EUID_ASSERT(); char *fname = check_dir_or_file(name); if (arg_debug) @@ -548,10 +548,10 @@ void fs_private_home_list(void) { selinux_relabel_path(RUN_HOME_DIR, homedir); fs_logger_print(); // save the current log + // copy the list of files in the new home directory + EUID_USER(); if (arg_debug) printf("Copying files in the new home:\n"); - - // copy the list of files in the new home directory char *dlist = strdup(cfg.home_private_keep); if (!dlist) errExit("strdup"); @@ -584,6 +584,7 @@ void fs_private_home_list(void) { exit(1); } // mount using the file descriptor + EUID_ROOT(); if (bind_mount_path_to_fd(RUN_HOME_DIR, fd)) errExit("mount bind"); close(fd); diff --git a/src/firejail/fs_lib.c b/src/firejail/fs_lib.c index 8369e6259..9d7a17cf3 100644 --- a/src/firejail/fs_lib.c +++ b/src/firejail/fs_lib.c @@ -178,8 +178,7 @@ void fslib_mount(const char *full_path) { if (*full_path == '\0' || !valid_full_path(full_path) || - access(full_path, F_OK) != 0 || - stat(full_path, &s) != 0 || + stat_as_user(full_path, &s) != 0 || s.st_uid != 0) return; @@ -246,7 +245,7 @@ static void load_library(const char *fname) { // existing file owned by root struct stat s; - if (!access(fname, F_OK) && stat(fname, &s) == 0 && s.st_uid == 0) { + if (stat_as_user(fname, &s) == 0 && s.st_uid == 0) { // load directories, regular 64 bit libraries, and 64 bit executables if (S_ISDIR(s.st_mode)) fslib_mount(fname); @@ -286,12 +285,14 @@ static void install_list_entry(const char *lib) { #define DO_GLOBBING #ifdef DO_GLOBBING // globbing + EUID_USER(); glob_t globbuf; int globerr = glob(fname, GLOB_NOCHECK | GLOB_NOSORT | GLOB_PERIOD, NULL, &globbuf); if (globerr) { fprintf(stderr, "Error: failed to glob private-lib pattern %s\n", fname); exit(1); } + EUID_ROOT(); size_t j; for (j = 0; j < globbuf.gl_pathc; j++) { assert(globbuf.gl_pathv[j]); diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c index ec3fe4c5b..370035a4d 100644 --- a/src/firejail/fs_whitelist.c +++ b/src/firejail/fs_whitelist.c @@ -257,6 +257,7 @@ static void whitelist_symlink(const char *link, const char *target) { } static void globbing(const char *pattern) { + EUID_ASSERT(); assert(pattern); // globbing diff --git a/src/firejail/macros.c b/src/firejail/macros.c index bcac1feb4..cd29d8f85 100644 --- a/src/firejail/macros.c +++ b/src/firejail/macros.c @@ -149,6 +149,7 @@ static char *resolve_xdg(const char *var) { // returns mallocated memory static char *resolve_hardcoded(char *entries[]) { + EUID_ASSERT(); char *fname; struct stat s; diff --git a/src/firejail/mountinfo.c b/src/firejail/mountinfo.c index a700729d3..64a94bd84 100644 --- a/src/firejail/mountinfo.c +++ b/src/firejail/mountinfo.c @@ -22,7 +22,7 @@ #include #ifndef O_PATH -# define O_PATH 010000000 +#define O_PATH 010000000 #endif #define MAX_BUF 4096 @@ -153,6 +153,7 @@ MountData *get_last_mount(void) { // Extract the mount id from /proc/self/fdinfo and return it. int get_mount_id(const char *path) { + EUID_ASSERT(); assert(path); int fd = open(path, O_PATH|O_CLOEXEC); @@ -162,7 +163,9 @@ int get_mount_id(const char *path) { char *fdinfo; if (asprintf(&fdinfo, "/proc/self/fdinfo/%d", fd) == -1) errExit("asprintf"); + EUID_ROOT(); FILE *fp = fopen(fdinfo, "re"); + EUID_USER(); free(fdinfo); if (!fp) goto errexit; diff --git a/src/firejail/paths.c b/src/firejail/paths.c index b800fa944..d58a9d272 100644 --- a/src/firejail/paths.c +++ b/src/firejail/paths.c @@ -136,7 +136,7 @@ int program_in_path(const char *program) { // ('x' permission means something different for directories). // exec follows symlinks, so use stat, not lstat. struct stat st; - if (stat(scratch, &st)) { + if (stat_as_user(scratch, &st)) { perror(scratch); exit(1); } diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c index d2c0bcc19..37111324a 100644 --- a/src/firejail/sbox.c +++ b/src/firejail/sbox.c @@ -265,7 +265,6 @@ int sbox_run(unsigned filtermask, int num, ...) { } int sbox_run_v(unsigned filtermask, char * const arg[]) { - EUID_ROOT(); assert(arg); if (arg_debug) { @@ -285,6 +284,7 @@ int sbox_run_v(unsigned filtermask, char * const arg[]) { if (child < 0) errExit("fork"); if (child == 0) { + EUID_ROOT(); sbox_do_exec_v(filtermask, arg); } diff --git a/src/firejail/util.c b/src/firejail/util.c index 44f1cbd02..b8643ff60 100644 --- a/src/firejail/util.c +++ b/src/firejail/util.c @@ -417,6 +417,13 @@ int is_dir(const char *fname) { if (*fname == '\0') return 0; + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); + // if fname doesn't end in '/', add one int rv; struct stat s; @@ -432,6 +439,9 @@ int is_dir(const char *fname) { free(tmp); } + if (called_as_root) + EUID_ROOT(); + if (rv == -1) return 0; @@ -447,6 +457,14 @@ int is_link(const char *fname) { if (*fname == '\0') return 0; + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); + + // remove trailing '/' if any char *tmp = strdup(fname); if (!tmp) errExit("strdup"); @@ -456,9 +474,66 @@ int is_link(const char *fname) { ssize_t rv = readlink(tmp, &c, 1); free(tmp); + if (called_as_root) + EUID_ROOT(); + return (rv != -1); } +char *realpath_as_user(const char *fname) { + assert(fname); + + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); + + char *rv = realpath(fname, NULL); + + if (called_as_root) + EUID_ROOT(); + + return rv; +} + +int stat_as_user(const char *fname, struct stat *s) { + assert(fname); + + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); + + int rv = stat(fname, s); + + if (called_as_root) + EUID_ROOT(); + + return rv; +} + +int lstat_as_user(const char *fname, struct stat *s) { + assert(fname); + + int called_as_root = 0; + if (geteuid() == 0) + called_as_root = 1; + + if (called_as_root) + EUID_USER(); + + int rv = lstat(fname, s); + + if (called_as_root) + EUID_ROOT(); + + return rv; +} + // remove all slashes and single dots from the end of a path // for example /foo/bar///././. -> /foo/bar void trim_trailing_slash_or_dot(char *path) { @@ -891,14 +966,13 @@ static int remove_callback(const char *fpath, const struct stat *sb, int typefla int remove_overlay_directory(void) { EUID_ASSERT(); - struct stat s; sleep(1); char *path; if (asprintf(&path, "%s/.firejail", cfg.homedir) == -1) errExit("asprintf"); - if (lstat(path, &s) == 0) { + if (access(path, F_OK) == 0) { pid_t child = fork(); if (child < 0) errExit("fork"); @@ -909,6 +983,7 @@ int remove_overlay_directory(void) { fprintf(stderr, "Error: cannot open %s\n", path); _exit(1); } + struct stat s; if (fstat(fd, &s) == -1) errExit("fstat"); if (!S_ISDIR(s.st_mode)) { @@ -944,7 +1019,7 @@ int remove_overlay_directory(void) { // wait for the child to finish waitpid(child, NULL, 0); // check if ~/.firejail was deleted - if (stat(path, &s) == 0) + if (access(path, F_OK) == 0) return 1; } return 0; @@ -977,9 +1052,8 @@ void flush_stdin(void) { int create_empty_dir_as_user(const char *dir, mode_t mode) { assert(dir); mode &= 07777; - struct stat s; - if (stat(dir, &s)) { + if (access(dir, F_OK) != 0) { if (arg_debug) printf("Creating empty %s directory\n", dir); pid_t child = fork(); @@ -1001,7 +1075,7 @@ int create_empty_dir_as_user(const char *dir, mode_t mode) { _exit(0); } waitpid(child, NULL, 0); - if (stat(dir, &s) == 0) + if (access(dir, F_OK) == 0) return 1; } return 0; @@ -1113,8 +1187,11 @@ unsigned extract_timeout(const char *str) { void disable_file_or_dir(const char *fname) { assert(fname); + assert(geteuid() == 0); + EUID_USER(); int fd = open(fname, O_PATH|O_CLOEXEC); + EUID_ROOT(); if (fd < 0) return; diff --git a/src/firejail/x11.c b/src/firejail/x11.c index afe77e246..09956b903 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -204,7 +204,6 @@ static int random_display_number(void) { void x11_start_xvfb(int argc, char **argv) { EUID_ASSERT(); int i; - struct stat s; pid_t jail = 0; pid_t server = 0; @@ -348,7 +347,7 @@ void x11_start_xvfb(int argc, char **argv) { // wait for x11 server to start while (++n < 10) { sleep(1); - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) break; }; @@ -427,7 +426,6 @@ static char *extract_setting(int argc, char **argv, const char *argument) { void x11_start_xephyr(int argc, char **argv) { EUID_ASSERT(); int i; - struct stat s; pid_t jail = 0; pid_t server = 0; @@ -586,7 +584,7 @@ void x11_start_xephyr(int argc, char **argv) { // wait for x11 server to start while (++n < 10) { sleep(1); - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) break; }; @@ -701,7 +699,6 @@ static char * get_title_arg_str() { static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, int display, char *display_str) { EUID_ASSERT(); int i; - struct stat s; pid_t client = 0; pid_t server = 0; @@ -818,7 +815,7 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, // wait for x11 server to start while (++n < 10) { sleep(1); - if (stat(fname, &s) == 0) + if (access(fname, F_OK) == 0) break; } @@ -1231,9 +1228,9 @@ void x11_xorg(void) { char *dest; if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1) errExit("asprintf"); - if (lstat(dest, &s) == -1) { + if (access(dest, F_OK) == -1) { touch_file_as_user(dest, 0600); - if (stat(dest, &s) == -1) { + if (access(dest, F_OK) == -1) { fprintf(stderr, "Error: cannot create %s\n", dest); exit(1); } @@ -1292,7 +1289,7 @@ void x11_xorg(void) { // blacklist user .Xauthority file if it is not masked already const char *envar = env_get("XAUTHORITY"); if (envar) { - char *rp = realpath(envar, NULL); + char *rp = realpath_as_user(envar); if (rp) { if (strcmp(rp, dest) != 0) disable_file_or_dir(rp); -- cgit v1.2.3