1 files changed, 114 insertions, 96 deletions

diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index 09de11de9..01182bd2c 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -54,16 +54,10 @@ 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 <OPERATION_MAX);
-        last_disable = UNSUCCESSFUL;
+        EUID_ASSERT();
 
         // Resolve all symlinks
         char* fname = realpath(filename, NULL);
@@ -71,20 +65,24 @@ static void disable_file(OPERATION op, const char *filename) {
                 return;
         }
         if (fname == NULL && errno == EACCES) {
-                if (arg_debug)
-                        printf("Debug: no access to file %s, forcing mount\n", filename);
                 // realpath and stat functions will fail on FUSE filesystems
                 // they don't seem to like a uid of 0
                 // force mounting
-                int rv = mount(RUN_RO_DIR, filename, "none", MS_BIND, "mode=400,gid=0");
-                if (rv == 0)
-                        last_disable = SUCCESSFUL;
-                else {
-                        rv = mount(RUN_RO_FILE, filename, "none", MS_BIND, "mode=400,gid=0");
-                        if (rv == 0)
-                                last_disable = SUCCESSFUL;
+                int fd = open(filename, O_PATH|O_CLOEXEC);
+                if (fd < 0) {
+                        if (arg_debug)
+                                printf("Warning (blacklisting): cannot open %s: %s\n", filename, strerror(errno));
+                        return;
                 }
-                if (last_disable == SUCCESSFUL) {
+
+                EUID_ROOT();
+                int err = bind_mount_path_to_fd(RUN_RO_DIR, fd);
+                if (err < 0)
+                        err = bind_mount_path_to_fd(RUN_RO_FILE, fd);
+                EUID_USER();
+                close(fd);
+
+                if (err == 0) {
                         if (arg_debug)
                                 printf("Disable %s\n", filename);
                         if (op == BLACKLIST_FILE)
@@ -92,21 +90,18 @@ static void disable_file(OPERATION op, const char *filename) {
                         else
                                 fs_logger2("blacklist-nolog", filename);
                 }
-                else {
-                        if (arg_debug)
-                                printf("Warning (blacklisting): %s is an invalid file, skipping...\n", filename);
-                }
+                else if (arg_debug)
+                        printf("Warning (blacklisting): cannot mount on %s\n", filename);
 
                 return;
         }
 
         // if the file is not present, do nothing
+        assert(fname);
         struct stat s;
-        if (fname == NULL)
-                return;
-        if (stat(fname, &s) == -1) {
+        if (stat(fname, &s) < 0) {
                 if (arg_debug)
-                        fwarning("%s does not exist, skipping...\n", fname);
+                        printf("Warning (blacklisting): cannot access %s: %s\n", fname, strerror(errno));
                 free(fname);
                 return;
         }
@@ -115,8 +110,10 @@ static void disable_file(OPERATION op, const char *filename) {
         // we migth have a file found in ${PATH} pointing to /usr/bin/firejail
         // blacklisting it here will end up breaking situations like user clicks on a link in Thunderbird
         //     and expects Firefox to open in the same sandbox
-        if (strcmp(BINDIR "/firejail", fname) == 0)
+        if (strcmp(BINDIR "/firejail", fname) == 0) {
+                free(fname);
                 return;
+        }
 
         // modify the file
         if (op == BLACKLIST_FILE || op == BLACKLIST_NOLOG) {
@@ -141,15 +138,25 @@ static void disable_file(OPERATION op, const char *filename) {
                                         printf(" - no logging\n");
                         }
 
+                        int fd = open(fname, O_PATH|O_CLOEXEC);
+                        if (fd < 0) {
+                                if (arg_debug)
+                                        printf("Warning (blacklisting): cannot open %s: %s\n", fname, strerror(errno));
+                                free(fname);
+                                return;
+                        }
+                        EUID_ROOT();
                         if (S_ISDIR(s.st_mode)) {
-                                if (mount(RUN_RO_DIR, fname, "none", MS_BIND, "mode=400,gid=0") < 0)
+                                if (bind_mount_path_to_fd(RUN_RO_DIR, fd) < 0)
                                         errExit("disable file");
                         }
                         else {
-                                if (mount(RUN_RO_FILE, fname, "none", MS_BIND, "mode=400,gid=0") < 0)
+                                if (bind_mount_path_to_fd(RUN_RO_FILE, fd) < 0)
                                         errExit("disable file");
                         }
-                        last_disable = SUCCESSFUL;
+                        EUID_USER();
+                        close(fd);
+
                         if (op == BLACKLIST_FILE)
                                 fs_logger2("blacklist", fname);
                         else
@@ -158,7 +165,6 @@ static void disable_file(OPERATION op, const char *filename) {
         }
         else if (op == MOUNT_READONLY || op == MOUNT_RDWR || op == MOUNT_NOEXEC) {
                 fs_remount_rec(fname, op);
-                // todo: last_disable = SUCCESSFUL;
         }
         else if (op == MOUNT_TMPFS) {
                 if (S_ISDIR(s.st_mode)) {
@@ -169,9 +175,10 @@ static void disable_file(OPERATION op, const char *filename) {
                                         exit(1);
                                 }
                         }
+                        // fs_tmpfs returns with EUID 0
                         fs_tmpfs(fname, getuid());
                         selinux_relabel_path(fname, fname);
-                        last_disable = SUCCESSFUL;
+                        EUID_USER();
                 }
                 else
                         fwarning("%s is not a directory; cannot mount a tmpfs on top of it.\n", fname);
@@ -191,6 +198,7 @@ static int *nbcheck = NULL;
 // Treat pattern as a shell glob pattern and blacklist matching files
 static void globbing(OPERATION op, const char *pattern, const char *noblacklist[], size_t noblacklist_len) {
         assert(pattern);
+        EUID_ASSERT();
 
 #ifdef TEST_NO_BLACKLIST_MATCHING
         if (nbcheck_start == 0) {
@@ -264,6 +272,7 @@ void fs_blacklist(void) {
         if (noblacklist == NULL)
                 errExit("failed allocating memory for noblacklist entries");
 
+        EUID_USER();
         while (entry) {
                 OPERATION op = OPERATION_MAX;
                 char *ptr;
@@ -294,11 +303,13 @@ void fs_blacklist(void) {
                         if (arg_debug)
                                 printf("Mount-bind %s on top of %s\n", dname1, dname2);
                         // preserve dname2 mode and ownership
+                        // EUID_ROOT(); - option not accessible to non-root users
                         if (mount(dname1, dname2, NULL, MS_BIND|MS_REC, NULL) < 0)
                                 errExit("mount bind");
                         /* coverity[toctou] */
                         if (set_perms(dname2,  s.st_uid, s.st_gid,s.st_mode))
                                 errExit("set_perms");
+                        // EUID_USER();
 
                         entry = entry->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,38 +501,42 @@ 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
-        int fd1 = safer_openat(-1, path, O_PATH|O_NOFOLLOW|O_CLOEXEC);
-        if (fd1 == -1)
+        int fd = safer_openat(-1, path, O_PATH|O_NOFOLLOW|O_CLOEXEC);
+        if (fd < 0)
                 goto out;
-        struct stat s1;
-        if (fstat(fd1, &s1) == -1) {
+
+        struct stat s;
+        if (fstat(fd, &s) < 0) {
                 // fstat can fail with EACCES if path is a FUSE mount,
                 // mounted without 'allow_root' or 'allow_other'
                 if (errno != EACCES)
                         errExit("fstat");
-                close(fd1);
+                close(fd);
                 goto out;
         }
         // get mount flags
         struct statvfs buf;
-        if (fstatvfs(fd1, &buf) == -1)
-                errExit("fstatvfs");
+        if (fstatvfs(fd, &buf) < 0) {
+                close(fd);
+                goto out;
+        }
         unsigned long flags = buf.f_flag;
 
         // read-write option
         if (op == MOUNT_RDWR || op == MOUNT_RDWR_NOCHECK) {
                 // nothing to do if there is no read-only flag
                 if ((flags & MS_RDONLY) == 0) {
-                        close(fd1);
+                        close(fd);
                         return;
                 }
                 // allow only user owned directories, except the user is root
-                if (op != MOUNT_RDWR_NOCHECK && getuid() != 0 && s1.st_uid != getuid()) {
+                if (op != MOUNT_RDWR_NOCHECK && getuid() != 0 && s.st_uid != getuid()) {
                         fwarning("you are not allowed to change %s to read-write\n", path);
-                        close(fd1);
+                        close(fd);
                         return;
                 }
                 flags &= ~MS_RDONLY;
@@ -530,7 +545,7 @@ static void fs_remount_simple(const char *path, OPERATION op) {
         else if (op == MOUNT_NOEXEC) {
                 // nothing to do if path is mounted noexec already
                 if ((flags & (MS_NOEXEC|MS_NODEV|MS_NOSUID)) == (MS_NOEXEC|MS_NODEV|MS_NOSUID)) {
-                        close(fd1);
+                        close(fd);
                         return;
                 }
                 flags |= MS_NOEXEC|MS_NODEV|MS_NOSUID;
@@ -539,7 +554,7 @@ static void fs_remount_simple(const char *path, OPERATION op) {
         else if (op == MOUNT_READONLY) {
                 // nothing to do if path is mounted read-only already
                 if ((flags & MS_RDONLY) == MS_RDONLY) {
-                        close(fd1);
+                        close(fd);
                         return;
                 }
                 flags |= MS_RDONLY;
@@ -549,29 +564,37 @@ static void fs_remount_simple(const char *path, OPERATION op) {
 
         if (arg_debug)
                 printf("Mounting %s %s\n", opstr[op], path);
+
+        // make path a mount point:
         // mount --bind path path
-        char *proc;
-        if (asprintf(&proc, "/proc/self/fd/%d", fd1) == -1)
-                errExit("asprintf");
-        if (mount(proc, proc, NULL, MS_BIND|MS_REC, NULL) < 0)
-                errExit("mount");
-        free(proc);
+        EUID_ROOT();
+        int err = bind_mount_by_fd(fd, fd);
+        EUID_USER();
+        if (err) {
+                close(fd);
+                goto out;
+        }
 
-        // mount --bind -o remount,ro path
-        // need to open path again without following symbolic links
+        // remount the mount point
+        // need to open path again
         int fd2 = safer_openat(-1, path, O_PATH|O_NOFOLLOW|O_CLOEXEC);
-        if (fd2 == -1)
-                errExit("open");
+        close(fd); // earliest timepoint to close fd
+        if (fd2 < 0)
+                goto out;
+
+        // device and inode number should be the same
         struct stat s2;
-        if (fstat(fd2, &s2) == -1)
+        if (fstat(fd2, &s2) < 0)
                 errExit("fstat");
-        // device and inode number should be the same
-        if (s1.st_dev != s2.st_dev || s1.st_ino != s2.st_ino)
+        if (s.st_dev != s2.st_dev || s.st_ino != s2.st_ino)
                 errLogExit("invalid %s mount", opstr[op]);
-        if (asprintf(&proc, "/proc/self/fd/%d", fd2) == -1)
-                errExit("asprintf");
-        if (mount(NULL, proc, NULL, flags|MS_BIND|MS_REMOUNT, NULL) < 0)
-                errExit("mount");
+
+        EUID_ROOT();
+        err = remount_by_fd(fd2, flags);
+        EUID_USER();
+        close(fd2);
+        if (err)
+                goto out;
 
         // run a sanity check on /proc/self/mountinfo and confirm that target of the last
         // mount operation was path; if there are other mount points contained inside path,
@@ -582,10 +605,8 @@ static void fs_remount_simple(const char *path, OPERATION op) {
            (*(mptr->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:
@@ -594,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;
@@ -632,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)
@@ -640,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
@@ -653,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();
 }
 
 
@@ -674,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");
@@ -689,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");
@@ -739,12 +765,8 @@ 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) {
-                struct stat s;
-
                 char *fname;
                 if (asprintf(&fname, "/run/user/%d", getuid()) == -1)
                         errExit("asprintf");
@@ -755,8 +777,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 +786,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);
@@ -777,30 +797,26 @@ 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) {
-        struct stat s;
-
+        EUID_USER();
         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);
+        EUID_ROOT();
 }
 
 
@@ -862,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;
 
@@ -1221,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");
 
@@ -1241,8 +1259,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