6 files changed, 55 insertions, 111 deletions

diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index 6c87df1e9..21ab56bd8 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -29,16 +29,25 @@
 
 static void fs_rdwr(const char *dir);
 
+static void create_dir_as_root(const char *dir, mode_t mode) {
+        assert(dir);
+        if (arg_debug)
+                printf("Creating %s directory\n", dir);
+
+        if (mkdir(dir, mode) == -1)
+                errExit("mkdir");
+
+        ASSERT_PERMS(dir, 0, 0, mode);
+}
+
 static void create_empty_dir(void) {
         struct stat s;
         
         if (stat(RUN_RO_DIR, &s)) {
                 /* coverity[toctou] */
-                int rv = mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR);
-                if (rv == -1)
-                        errExit("mkdir");       
-                if (chown(RUN_RO_DIR, 0, 0) < 0)
-                        errExit("chown");
+                if (mkdir(RUN_RO_DIR, S_IRUSR | S_IXUSR) == -1)
+                        errExit("mkdir");
+                ASSERT_PERMS(RUN_RO_DIR, 0, 0, S_IRUSR | S_IXUSR);
         }
 }
 
@@ -50,11 +59,16 @@ static void create_empty_file(void) {
                 FILE *fp = fopen(RUN_RO_FILE, "w");
                 if (!fp)
                         errExit("fopen");
-                fclose(fp);
-                if (chown(RUN_RO_FILE, 0, 0) < 0)
+
+                int fd = fileno(fp);
+                if (fd == -1)
+                        errExit("fileno");
+                if (fchown(fd, 0, 0) < 0)
                         errExit("chown");
-                if (chmod(RUN_RO_FILE, S_IRUSR) < 0)
+                if (fchmod(fd, S_IRUSR) < 0)
                         errExit("chown");
+
+                fclose(fp);
         }
 }
 
@@ -64,16 +78,7 @@ void fs_build_firejail_dir(void) {
 
         // CentOS 6 doesn't have /run directory
         if (stat(RUN_FIREJAIL_BASEDIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_BASEDIR);
-                /* coverity[toctou] */
-                int rv = mkdir(RUN_FIREJAIL_BASEDIR, 0755);
-                if (rv == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_BASEDIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_BASEDIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_BASEDIR, 0755);
         }
         else { // check /tmp/firejail directory belongs to root end exit if doesn't!
                 if (s.st_uid != 0 || s.st_gid != 0) {
@@ -83,61 +88,23 @@ void fs_build_firejail_dir(void) {
         }
 
         if (stat(RUN_FIREJAIL_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_DIR);
-                /* coverity[toctou] */
-                int rv = mkdir(RUN_FIREJAIL_DIR, 0755);
-                if (rv == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_DIR, 0755);
         }
         
         if (stat(RUN_FIREJAIL_NETWORK_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_NETWORK_DIR);
-                
-                if (mkdir(RUN_FIREJAIL_NETWORK_DIR, 0755) == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_NETWORK_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_NETWORK_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_NETWORK_DIR, 0755);
         }
         
         if (stat(RUN_FIREJAIL_BANDWIDTH_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_BANDWIDTH_DIR);
-                if (mkdir(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_BANDWIDTH_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_BANDWIDTH_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_BANDWIDTH_DIR, 0755);
         }
                 
         if (stat(RUN_FIREJAIL_NAME_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_NAME_DIR);
-                if (mkdir(RUN_FIREJAIL_NAME_DIR, 0755) == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_NAME_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_NAME_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_NAME_DIR, 0755);
         }
         
         if (stat(RUN_FIREJAIL_X11_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_FIREJAIL_X11_DIR);
-                if (mkdir(RUN_FIREJAIL_X11_DIR, 0755) == -1)
-                        errExit("mkdir");
-                if (chown(RUN_FIREJAIL_X11_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_FIREJAIL_X11_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_FIREJAIL_X11_DIR, 0755);
         }
         
         create_empty_dir();
@@ -160,16 +127,7 @@ void fs_build_mnt_dir(void) {
         
         // create /run/firejail/mnt directory
         if (stat(RUN_MNT_DIR, &s)) {
-                if (arg_debug)
-                        printf("Creating %s directory\n", RUN_MNT_DIR);
-                /* coverity[toctou] */
-                int rv = mkdir(RUN_MNT_DIR, 0755);
-                if (rv == -1)
-                        errExit("mkdir");
-                if (chown(RUN_MNT_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_MNT_DIR, 0755) < 0)
-                        errExit("chmod");
+                create_dir_as_root(RUN_MNT_DIR, 0755);
         }
 
         // ... and mount tmpfs on top of it
@@ -202,16 +160,12 @@ void fs_build_cp_command(void) {
                         fprintf(stderr, "Error: invalid /bin/cp file\n");
                         exit(1);
                 }
-                int rv = copy_file(fname, RUN_CP_COMMAND);
+                int rv = copy_file(fname, RUN_CP_COMMAND, 0, 0, 0755);
                 if (rv) {
                         fprintf(stderr, "Error: cannot access /bin/cp\n");
                         exit(1);
                 }
-                /* coverity[toctou] */
-                if (chown(RUN_CP_COMMAND, 0, 0))
-                        errExit("chown");
-                if (chmod(RUN_CP_COMMAND, 0755))
-                        errExit("chmod");
+                ASSERT_PERMS(RUN_CP_COMMAND, 0, 0, 0755);
                         
                 free(fname);
         }
@@ -827,10 +781,7 @@ char *fs_check_overlay_dir(const char *subdirname, int allow_reuse) {
                 /* coverity[toctou] */
                 if (mkdir(dirname, 0700))
                         errExit("mkdir");
-                if (chown(dirname, getuid(), getgid()) < 0)
-                        errExit("chown");
-                if (chmod(dirname, 0700) < 0)
-                        errExit("chmod");
+                ASSERT_PERMS(dirname, getuid(), getgid(), 0700);
         }
         else if (is_link(dirname)) {
                 fprintf(stderr, "Error: invalid ~/.firejail directory\n");
@@ -917,10 +868,7 @@ void fs_overlayfs(void) {
                 errExit("asprintf");
         if (mkdir(oroot, 0755))
                 errExit("mkdir");
-        if (chown(oroot, 0, 0) < 0)
-                errExit("chown");
-        if (chmod(oroot, 0755) < 0)
-                errExit("chmod");
+        ASSERT_PERMS(oroot, 0, 0, 0755);
 
         struct stat s;
         char *basedir = RUN_MNT_DIR;
@@ -1259,7 +1207,7 @@ void fs_chroot(const char *rootdir) {
                         fprintf(stderr, "Error: invalid %s file\n", fname);
                         exit(1);
                 }
-                if (copy_file("/etc/resolv.conf", fname) == -1)
+                if (copy_file("/etc/resolv.conf", fname, 0, 0, 0644) == -1)
                         fprintf(stderr, "Warning: /etc/resolv.conf not initialized\n");
         }
         
diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c
index d328d5f1c..75d69e021 100644
--- a/src/firejail/fs_home.c
+++ b/src/firejail/fs_home.c
@@ -43,9 +43,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
                 if (stat(fname, &s) == 0)
                         return;
                 if (stat("/etc/skel/.zshrc", &s) == 0) {
-                        if (copy_file("/etc/skel/.zshrc", fname) == 0) {
-                                if (chown(fname, u, g) == -1)
-                                        errExit("chown");
+                        if (copy_file("/etc/skel/.zshrc", fname, u, g, 0644) == 0) {
                                 fs_logger("clone /etc/skel/.zshrc");
                         }
                 }
@@ -73,9 +71,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
                 if (stat(fname, &s) == 0)
                         return;
                 if (stat("/etc/skel/.cshrc", &s) == 0) {
-                        if (copy_file("/etc/skel/.cshrc", fname) == 0) {
-                                if (chown(fname, u, g) == -1)
-                                        errExit("chown");
+                        if (copy_file("/etc/skel/.cshrc", fname, u, g, 0644) == 0) {
                                 fs_logger("clone /etc/skel/.cshrc");
                         }
                 }
@@ -104,10 +100,7 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
                 if (stat(fname, &s) == 0) 
                         return;
                 if (stat("/etc/skel/.bashrc", &s) == 0) {
-                        if (copy_file("/etc/skel/.bashrc", fname) == 0) {
-                                /* coverity[toctou] */
-                                if (chown(fname, u, g) == -1)
-                                        errExit("chown");
+                        if (copy_file("/etc/skel/.bashrc", fname, u, g, 0644) == 0) {
                                 fs_logger("clone /etc/skel/.bashrc");
                         }
                 }
@@ -131,7 +124,7 @@ static int store_xauthority(void) {
                         exit(1);
                 }
                         
-                int rv = copy_file(src, dest);
+                int rv = copy_file(src, dest, -1, -1, 0600);
                 if (rv) {
                         fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n");
                         return 0;
@@ -167,7 +160,7 @@ static int store_asoundrc(void) {
                         free(rp);
                 }
 
-                int rv = copy_file(src, dest);
+                int rv = copy_file(src, dest, -1, -1, -0644);
                 if (rv) {
                         fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n");
                         return 0;
@@ -184,7 +177,7 @@ static void copy_xauthority(void) {
         char *dest;
         if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1)
                 errExit("asprintf");
-        int rv = copy_file(src, dest);
+        int rv = copy_file(src, dest, -1, -1, 0600);
         if (rv)
                 fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n");
         else {
@@ -207,7 +200,7 @@ static void copy_asoundrc(void) {
         char *dest;
         if (asprintf(&dest, "%s/.asoundrc", cfg.homedir) == -1)
                 errExit("asprintf");
-        int rv = copy_file(src, dest);
+        int rv = copy_file(src, dest, -1 , -1, 0644);
         if (rv)
                 fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n");
         else {
@@ -360,11 +353,9 @@ int fs_copydir(const char *path, const struct stat *st, int ftype, struct FTW *s
          return(0);
       if (stat(path, &s) == 0) {
          if(ftype == FTW_F) {
-            if (copy_file(path, dest) == 0) {
+            if (copy_file(path, dest, u, g, 0644) == 0) {
                if (arg_debug)
                   printf("copy from %s to %s\n", path, dest);
-               if (chown(dest, u, g) == -1)
-                  errExit("chown");
                fs_logger2("clone", path);
             }
          }
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index 09577fb0c..495aaf8e2 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -374,11 +374,7 @@ void sandboxfs(int op, pid_t pid, const char *path) {
                 }
                 // copy file
                 EUID_ROOT();
-                copy_file(src_fname, dest_fname);
-                if (chown(dest_fname, getuid(), getgid()) == -1)
-                        errExit("chown");
-                if (chmod(dest_fname, 0644) == -1)
-                        errExit("chmod");
+                copy_file(src_fname, dest_fname, getuid(), getgid(), 0644);
                 printf("Transfer complete\n");
                 EUID_USER();
         }
diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c
index 908ef1d25..dd26d219c 100644
--- a/src/firejail/pulseaudio.c
+++ b/src/firejail/pulseaudio.c
@@ -114,7 +114,7 @@ void pulseaudio_init(void) {
         char *pulsecfg = NULL;
         if (asprintf(&pulsecfg, "%s/client.conf", RUN_PULSE_DIR) == -1)
                 errExit("asprintf");
-        if (copy_file("/etc/pulse/client.conf", pulsecfg))
+        if (copy_file("/etc/pulse/client.conf", pulsecfg, -1, -1, 0644))
                 errExit("copy_file");
         FILE *fp = fopen(pulsecfg, "a+");
         if (!fp)
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 24bb71e4c..22434e200 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -170,7 +170,7 @@ void logerr(const char *msg) {
 
 
 // return -1 if error, 0 if no error
-int copy_file(const char *srcname, const char *destname) {
+int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, mode_t mode) {
         assert(srcname);
         assert(destname);
 
@@ -207,6 +207,11 @@ int copy_file(const char *srcname, const char *destname) {
                 }
         }
 
+        if (fchown(dst, uid, gid) == -1)
+                errExit("fchown");
+        if (fchmod(dst, mode) == -1)
+                errExit("fchmod");
+
         close(src);
         close(dst);
         return 0;
diff --git a/src/include/euid_common.h b/src/include/euid_common.h
index b6d341bf4..de5572fb1 100644
--- a/src/include/euid_common.h
+++ b/src/include/euid_common.h
@@ -37,11 +37,15 @@ extern uid_t firejail_uid;
 static inline void EUID_ROOT(void) {
         if (seteuid(0) == -1)
                 fprintf(stderr, "Warning: cannot switch euid to root\n");
+        if (setegid(0) == -1)
+                fprintf(stderr, "Warning: cannot switch egid to root\n");
 }
 
 static inline void EUID_USER(void) {
         if (seteuid(firejail_uid) == -1)
-                fprintf(stderr, "Warning: cannot switch euid to user\n");
+                errExit("seteuid");
+        if (setegid(firejail_uid) == -1)
+                errExit("setegid");
 }
 
 static inline void EUID_PRINT(void) {