4 files changed, 50 insertions, 27 deletions

diff --git a/src/firecfg/main.c b/src/firecfg/main.c
index 810af6ff2..84f6a5f77 100644
--- a/src/firecfg/main.c
+++ b/src/firecfg/main.c
@@ -379,6 +379,8 @@ int main(int argc, char **argv) {
                                 exit(1);
                         }
 
+                        // set umask, access database must be world-readable
+                        umask(022);
                         for (j = i + 1; j < argc; j++) {
                                 printf("Adding user %s to Firejail access database in %s/firejail.users\n", argv[j], SYSCONFDIR);
                                 firejail_user_add(argv[j]);
@@ -433,7 +435,10 @@ int main(int argc, char **argv) {
         // add user to firejail access database - only for root
         if (getuid() == 0) {
                 printf("\nAdding user %s to Firejail access database in %s/firejail.users\n", user, SYSCONFDIR);
+                // temporarily set the umask, access database must be world-readable
+                mode_t orig_umask = umask(022);
                 firejail_user_add(user);
+                umask(orig_umask);
         }
 
         // set new symlinks based on ~/.config/firejail directory
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 8a397e3d8..2e921ad37 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -562,7 +562,7 @@ typedef struct {
 // mountinfo.c
 MountData *get_last_mount(void);
 int get_mount_id(const char *path);
-char **build_mount_array(const int mountid, const char *path);
+char **build_mount_array(const int mount_id, const char *path);
 
 // fs_var.c
 void fs_var_log(void);  // mounting /var/log
diff --git a/src/firejail/mountinfo.c b/src/firejail/mountinfo.c
index ab1e501a7..c89845ace 100644
--- a/src/firejail/mountinfo.c
+++ b/src/firejail/mountinfo.c
@@ -69,7 +69,7 @@ static void unmangle_path(char *path) {
 static void parse_line(char *line, MountData *output) {
         assert(line && output);
         memset(output, 0, sizeof(*output));
-        // extract filesystem name, directory and filesystem types
+        // extract mount id, filesystem name, directory and filesystem types
         // examples:
         //      587 543 8:1 /tmp /etc rw,relatime master:1 - ext4 /dev/sda1 rw,errors=remount-ro,data=ordered
         //              output.mountid: 587
@@ -153,6 +153,8 @@ MountData *get_last_mount(void) {
 
 // Extract the mount id from /proc/self/fdinfo and return it.
 int get_mount_id(const char *path) {
+        assert(path);
+
         int fd = open(path, O_PATH|O_CLOEXEC);
         if (fd == -1)
                 return -1;
@@ -197,8 +199,12 @@ int get_mount_id(const char *path) {
         return -2;
 }
 
-// Return array with all paths that might need a remount.
-char **build_mount_array(const int mountid, const char *path) {
+// Check /proc/self/mountinfo if path has any submounts (or if path would have submounts
+// if it was made a mount point).
+// Returns an array that can be iterated over for recursive remounting.
+char **build_mount_array(const int mount_id, const char *path) {
+        assert(path);
+
         // open /proc/self/mountinfo
         FILE *fp = fopen("/proc/self/mountinfo", "re");
         if (!fp) {
@@ -207,30 +213,34 @@ char **build_mount_array(const int mountid, const char *path) {
                 exit(1);
         }
 
-        size_t size = 32;
+        // array to be returned
         size_t cnt = 0;
+        size_t size = 32;
         char **rv = malloc(size * sizeof(*rv));
         if (!rv)
                 errExit("malloc");
 
         // read /proc/self/mountinfo
         size_t pathlen = strlen(path);
+        char buf[MAX_BUF];
+        MountData mntp;
         int found = 0;
-        if (fgets(mbuf, MAX_BUF, fp) == NULL) {
+
+        if (fgets(buf, MAX_BUF, fp) == NULL) {
                 fprintf(stderr, "Error: cannot read /proc/self/mountinfo\n");
                 exit(1);
         }
         do {
                 // find mount point with mount id
                 if (!found) {
-                        parse_line(mbuf, &mdata);
-                        if (mdata.mountid == mountid) {
-                            // don't remount blacklisted paths,
-                            // give up if mount id has been reassigned
-                            if (strstr(mdata.fsname, "firejail.ro.dir") ||
-                                strstr(mdata.fsname, "firejail.ro.file") ||
-                                strncmp(mdata.dir, path, strlen(mdata.dir)))
-                                break;
+                        parse_line(buf, &mntp);
+                        if (mntp.mountid == mount_id) {
+                                // give up if mount id has been reassigned,
+                                // don't remount blacklisted path
+                                if (strncmp(mntp.dir, path, strlen(mntp.dir)) ||
+                                    strstr(mntp.fsname, "firejail.ro.dir") ||
+                                    strstr(mntp.fsname, "firejail.ro.file"))
+                                            break;
 
                                 *rv = strdup(path);
                                 if (*rv == NULL)
@@ -244,24 +254,24 @@ char **build_mount_array(const int mountid, const char *path) {
                 }
                 // from here on add all mount points below path,
                 // don't remount blacklisted paths
-                parse_line(mbuf, &mdata);
-                if (strncmp(mdata.dir, path, pathlen) == 0 &&
-                    mdata.dir[pathlen] == '/' &&
-                    strstr(mdata.fsname, "firejail.ro.dir") == NULL &&
-                    strstr(mdata.fsname, "firejail.ro.file") == NULL) {
+                parse_line(buf, &mntp);
+                if (strncmp(mntp.dir, path, pathlen) == 0 &&
+                    mntp.dir[pathlen] == '/' &&
+                    strstr(mntp.fsname, "firejail.ro.dir") == NULL &&
+                    strstr(mntp.fsname, "firejail.ro.file") == NULL) {
 
-                        if (cnt >= size) {
+                        if (cnt == size) {
                                 size *= 2;
                                 rv = realloc(rv, size * sizeof(*rv));
                                 if (!rv)
                                         errExit("realloc");
                         }
-                        rv[cnt] = strdup(mdata.dir);
+                        rv[cnt] = strdup(mntp.dir);
                         if (rv[cnt] == NULL)
                                 errExit("strdup");
                         cnt++;
                 }
-        } while (fgets(mbuf, MAX_BUF, fp));
+        } while (fgets(buf, MAX_BUF, fp));
 
         if (cnt == size) {
                 size++;
diff --git a/src/lib/firejail_user.c b/src/lib/firejail_user.c
index b270db459..a595d8331 100644
--- a/src/lib/firejail_user.c
+++ b/src/lib/firejail_user.c
@@ -29,6 +29,7 @@
 #include "../include/firejail_user.h"
 #include <sys/types.h>
 #include <pwd.h>
+#include <errno.h>
 
 #define MAXBUF 4098
 
@@ -113,15 +114,20 @@ int firejail_user_check(const char *name) {
 
         // check file existence
         char *fname = get_fname();
-        if (access(fname, F_OK)) {
+        assert(fname);
+        if (access(fname, F_OK) == -1 && errno == ENOENT) {
+                // assume the user doesn't care about access checking
                 free(fname);
-                return 1;       // assume the user doesn't care about access checking
+                return 1;
         }
 
         FILE *fp = fopen(fname, "r");
+        if (!fp) {
+                fprintf(stderr, "Error: cannot read %s\n", fname);
+                perror("fopen");
+                exit(1);
+        }
         free(fname);
-        if (!fp)
-                return 0;
 
         char buf[MAXBUF];
         while (fgets(buf, MAXBUF, fp)) {
@@ -162,11 +168,13 @@ void firejail_user_add(const char *name) {
         if (access(fname, F_OK) == 0) {
                 if (firejail_user_check(name)) {
                         printf("User %s already in the database\n", name);
+                        free(fname);
                         return;
                 }
         }
+        else
+                printf("Creating %s\n", fname);
 
-        printf("%s created\n", fname);
         FILE *fp = fopen(fname, "a+");
         if (!fp) {
                 fprintf(stderr, "Error: cannot open %s\n", fname);