tighten security

11 files changed, 48 insertions, 132 deletions

diff --git a/src/firejail/bandwidth.c b/src/firejail/bandwidth.c
index 5ff67b644..22be5b23c 100644
--- a/src/firejail/bandwidth.c
+++ b/src/firejail/bandwidth.c
@@ -130,14 +130,8 @@ static void bandwidth_create_run_file(pid_t pid) {
         /* coverity[toctou] */
         FILE *fp = fopen(fname, "w");
         if (fp) {
+                SET_PERMS_STREAM(fp, 0, 0, 0644);
                 fclose(fp);
-        
-                /* coverity[toctou] */
-                if (chmod(fname, 0644) == -1)
-                        errExit("chmod");
-                /* coverity[toctou] */
-                if (chown(fname, 0, 0) == -1)
-                        errExit("chown");
         }
         else {
                 fprintf(stderr, "Error: cannot create bandwidth file\n");
@@ -180,12 +174,9 @@ void network_set_run_file(pid_t pid) {
                         fprintf(fp, "%s:%s\n", cfg.bridge2.dev, cfg.bridge2.devsandbox);
                 if (cfg.bridge3.configured)
                         fprintf(fp, "%s:%s\n", cfg.bridge3.dev, cfg.bridge3.devsandbox);
-                fclose(fp);
 
-                if (chmod(fname, 0644) == -1)
-                        errExit("chmod");
-                if (chown(fname, 0, 0) == -1)
-                        errExit("chown");
+                SET_PERMS_STREAM(fp, 0, 0, 0644);
+                fclose(fp);
         }
         else {
                 fprintf(stderr, "Error: cannot create network map file\n");
diff --git a/src/firejail/cgroup.c b/src/firejail/cgroup.c
index ebd87f0d2..d9c7af9cf 100644
--- a/src/firejail/cgroup.c
+++ b/src/firejail/cgroup.c
@@ -30,10 +30,9 @@ void save_cgroup(void) {
         if (fp) {
                 fprintf(fp, "%s", cfg.cgroup);
                 fflush(0);
+                SET_PERMS_STREAM(fp, 0, 0, 0644);
                 if (fclose(fp))
                         goto errout;
-                if (chown(RUN_CGROUP_CFG, 0, 0) < 0)
-                        errExit("chown");
         }
         else
                 goto errout;
diff --git a/src/firejail/cpu.c b/src/firejail/cpu.c
index 1802ad5e1..cfb03e5fc 100644
--- a/src/firejail/cpu.c
+++ b/src/firejail/cpu.c
@@ -78,11 +78,8 @@ void save_cpu(void) {
         FILE *fp = fopen(RUN_CPU_CFG, "w");
         if (fp) {
                 fprintf(fp, "%x\n", cfg.cpus);
+                SET_PERMS_STREAM(fp, 0, 0, 0600);
                 fclose(fp);
-                if (chmod(RUN_CPU_CFG, 0600) < 0)
-                        errExit("chmod");
-                if (chown(RUN_CPU_CFG, 0, 0) < 0)
-                        errExit("chown");
         }
         else {
                 fprintf(stderr, "Error: cannot save cpu affinity mask\n");
diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index 21ab56bd8..27c69d0e1 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -60,14 +60,7 @@ static void create_empty_file(void) {
                 if (!fp)
                         errExit("fopen");
 
-                int fd = fileno(fp);
-                if (fd == -1)
-                        errExit("fileno");
-                if (fchown(fd, 0, 0) < 0)
-                        errExit("chown");
-                if (fchmod(fd, S_IRUSR) < 0)
-                        errExit("chown");
-
+                SET_PERMS_STREAM(fp, 0, 0, S_IRUSR);
                 fclose(fp);
         }
 }
diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c
index 6bd407346..b9d8614d4 100644
--- a/src/firejail/fs_bin.c
+++ b/src/firejail/fs_bin.c
@@ -203,14 +203,9 @@ void fs_private_bin_list(void) {
         
         // create /tmp/firejail/mnt/bin directory
         fs_build_mnt_dir();
-        int rv = mkdir(RUN_BIN_DIR, 0755);
-        if (rv == -1)
+        if (mkdir(RUN_BIN_DIR, 0755) == -1)
                 errExit("mkdir");
-        if (chown(RUN_BIN_DIR, 0, 0) < 0)
-                errExit("chown");
-        if (chmod(RUN_BIN_DIR, 0755) < 0)
-                errExit("chmod");
-        
+        ASSERT_PERMS(RUN_BIN_DIR, 0, 0, 0755);
         
         // copy the list of files in the new etc directory
         // using a new child process without root privileges
diff --git a/src/firejail/fs_dev.c b/src/firejail/fs_dev.c
index c7a27115f..571848dec 100644
--- a/src/firejail/fs_dev.c
+++ b/src/firejail/fs_dev.c
@@ -32,15 +32,11 @@
 
 static void create_char_dev(const char *path, mode_t mode, int major, int minor) {
         dev_t dev = makedev(major, minor);
-        int rv = mknod(path, S_IFCHR | mode, dev);
-        if (rv == -1)
+        if (mknod(path, S_IFCHR | mode, dev) == -1)
                 goto errexit;
-        
-
         if (chmod(path, mode) < 0)
                 goto errexit;
-        if (chown(path, 0, 0) < 0)
-                goto errexit;
+        ASSERT_PERMS(path, 0, 0, mode);
 
         return;
         
@@ -78,14 +74,9 @@ void fs_private_dev(void){
         // create DRI_DIR
         fs_build_mnt_dir();
         if (have_dri) {
-                /* coverity[toctou] */
-                rv = mkdir(RUN_DRI_DIR, 0755);
-                if (rv == -1)
+                if (mkdir(RUN_DRI_DIR, 0755) == -1)
                         errExit("mkdir");
-                if (chown(RUN_DRI_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_DRI_DIR, 0755) < 0)
-                        errExit("chmod");
+                ASSERT_PERMS(RUN_DRI_DIR, 0, 0, 0755);
         
                 // keep a copy of /dev/dri under DRI_DIR
                 if (mount("/dev/dri", RUN_DRI_DIR, NULL, MS_BIND|MS_REC, NULL) < 0)
@@ -94,14 +85,9 @@ void fs_private_dev(void){
         
         // create SND_DIR
         if (have_snd) {
-                /* coverity[toctou] */
-                rv = mkdir(RUN_SND_DIR, 0755);
-                if (rv == -1)
+                if (mkdir(RUN_SND_DIR, 0755) == -1)
                         errExit("mkdir");
-                if (chown(RUN_SND_DIR, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_SND_DIR, 0755) < 0)
-                        errExit("chmod");
+                ASSERT_PERMS(RUN_SND_DIR, 0, 0, 0755);
         
                 // keep a copy of /dev/dri under DRI_DIR
                 if (mount("/dev/snd", RUN_SND_DIR, NULL, MS_BIND|MS_REC, NULL) < 0)
@@ -143,13 +129,9 @@ void fs_private_dev(void){
         // bring back the /dev/snd directory
         if (have_snd) {
                 /* coverity[toctou] */
-                rv = mkdir("/dev/snd", 0755);
-                if (rv == -1)
+                if (mkdir("/dev/snd", 0755) == -1)
                         errExit("mkdir");
-                if (chown("/dev/snd", 0, 0) < 0)
-                        errExit("chown");
-                if (chmod("/dev/snd",0755) < 0)
-                        errExit("chmod");
+                ASSERT_PERMS("/dev/snd", 0, 0, 0755);
                 if (mount(RUN_SND_DIR, "/dev/snd", NULL, MS_BIND|MS_REC, NULL) < 0)
                         errExit("mounting /dev/snd");
                 fs_logger("whitelist /dev/snd");
@@ -157,14 +139,9 @@ void fs_private_dev(void){
 
         // bring back the /dev/dri directory
         if (have_dri) {
-                /* coverity[toctou] */
-                rv = mkdir("/dev/dri", 0755);
-                if (rv == -1)
+                if (mkdir("/dev/dri", 0755) == -1)
                         errExit("mkdir");
-                if (chown("/dev/dri", 0, 0) < 0)
-                        errExit("chown");
-                if (chmod("/dev/dri",0755) < 0)
-                        errExit("chmod");
+                ASSERT_PERMS("/dev/dri", 0, 0, 0755);
                 if (mount(RUN_DRI_DIR, "/dev/dri", NULL, MS_BIND|MS_REC, NULL) < 0)
                         errExit("mounting /dev/dri");
                 fs_logger("whitelist /dev/dri");
@@ -173,13 +150,12 @@ void fs_private_dev(void){
         // create /dev/shm
         if (arg_debug)
                 printf("Create /dev/shm directory\n");
-        rv = mkdir("/dev/shm", 01777);
-        if (rv == -1)
+        if (mkdir("/dev/shm", 01777) == -1)
                 errExit("mkdir");
-        if (chown("/dev/shm", 0, 0) < 0)
-                errExit("chown");
+        // mkdir sets only the file permission bits
         if (chmod("/dev/shm", 01777) < 0)
                 errExit("chmod");
+        ASSERT_PERMS("/dev/shm", 0, 0, 01777);
         fs_logger("mkdir /dev/shm");
 
         // create devices
@@ -201,13 +177,9 @@ void fs_private_dev(void){
 #endif
 
         // pseudo-terminal
-        rv = mkdir("/dev/pts", 0755);
-        if (rv == -1)
+        if (mkdir("/dev/pts", 0755) == -1)
                 errExit("mkdir");
-        if (chown("/dev/pts", 0, 0) < 0)
-                errExit("chown");
-        if (chmod("/dev/pts", 0755) < 0)
-                errExit("chmod");
+        ASSERT_PERMS("/dev/pts", 0, 0, 0755);
         fs_logger("mkdir /dev/pts");
         create_char_dev("/dev/pts/ptmx", 0666, 5, 2); //"mknod -m 666 /dev/pts/ptmx c 5 2");
         fs_logger("mknod /dev/pts/ptmx");
@@ -258,10 +230,10 @@ void fs_dev_shm(void) {
                                 // create directory
                                 if (mkdir(lnk, 01777))
                                         errExit("mkdir");
-                                if (chown(lnk, 0, 0))
-                                        errExit("chown");
+                                // mkdir sets only the file permission bits
                                 if (chmod(lnk, 01777))
                                         errExit("chmod");
+                                ASSERT_PERMS(lnk, 0, 0, 01777);
                         }
                         if (arg_debug)
                                 printf("Mounting tmpfs on %s on behalf of /dev/shm\n", lnk);
diff --git a/src/firejail/fs_etc.c b/src/firejail/fs_etc.c
index 6b9a4395b..d5b348ee2 100644
--- a/src/firejail/fs_etc.c
+++ b/src/firejail/fs_etc.c
@@ -130,13 +130,9 @@ void fs_private_etc_list(void) {
 
         // create /tmp/firejail/mnt/etc directory
         fs_build_mnt_dir();
-        int rv = mkdir(RUN_ETC_DIR, 0755);
-        if (rv == -1)
+        if (mkdir(RUN_ETC_DIR, 0755) == -1)
                 errExit("mkdir");
-        if (chown(RUN_ETC_DIR, 0, 0) < 0)
-                errExit("chown");
-        if (chmod(RUN_ETC_DIR, 0755) < 0)
-                errExit("chmod");
+        ASSERT_PERMS(RUN_ETC_DIR, 0, 0, 0755);
         fs_logger("tmpfs /etc");
         
         fs_logger_print();      // save the current log
diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c
index 75d69e021..85fa244be 100644
--- a/src/firejail/fs_home.c
+++ b/src/firejail/fs_home.c
@@ -51,11 +51,8 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
                         FILE *fp = fopen(fname, "w");
                         if (fp) {
                                 fprintf(fp, "\n");
+                                SET_PERMS_STREAM(fp, u, g, S_IRUSR | S_IWUSR);
                                 fclose(fp);
-                                if (chown(fname, u, g) == -1)
-                                        errExit("chown");
-                                if (chmod(fname, S_IRUSR | S_IWUSR) < 0)
-                                        errExit("chown");
                                 fs_logger2("touch", fname);
                         }
                 }
@@ -80,11 +77,8 @@ static void skel(const char *homedir, uid_t u, gid_t g) {
                         FILE *fp = fopen(fname, "w");
                         if (fp) {
                                 fprintf(fp, "\n");
+                                SET_PERMS_STREAM(fp, u, g, S_IRUSR | S_IWUSR);
                                 fclose(fp);
-                                if (chown(fname, u, g) == -1)
-                                        errExit("chown");
-                                if (chmod(fname, S_IRUSR | S_IWUSR) < 0)
-                                        errExit("chown");
                                 fs_logger2("touch", fname);
                         }
                 }
@@ -177,17 +171,12 @@ static void copy_xauthority(void) {
         char *dest;
         if (asprintf(&dest, "%s/.Xauthority", cfg.homedir) == -1)
                 errExit("asprintf");
-        int rv = copy_file(src, dest, -1, -1, 0600);
+        // copy, set permissions and ownership
+        int rv = copy_file(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR);
         if (rv)
                 fprintf(stderr, "Warning: cannot transfer .Xauthority in private home directory\n");
         else {
                 fs_logger2("clone", dest);
-        
-                // set permissions and ownership
-                if (chown(dest, getuid(), getgid()) < 0)
-                        errExit("chown");
-                if (chmod(dest, S_IRUSR | S_IWUSR) < 0)
-                        errExit("chmod");
         }
         
         // delete the temporary file
@@ -200,17 +189,12 @@ static void copy_asoundrc(void) {
         char *dest;
         if (asprintf(&dest, "%s/.asoundrc", cfg.homedir) == -1)
                 errExit("asprintf");
-        int rv = copy_file(src, dest, -1 , -1, 0644);
+        // copy, set permissions and ownership
+        int rv = copy_file(src, dest, getuid(), getgid(), S_IRUSR | S_IWUSR);
         if (rv)
                 fprintf(stderr, "Warning: cannot transfer .asoundrc in private home directory\n");
         else {
                 fs_logger2("clone", dest);
-        
-                // set permissions and ownership
-                if (chown(dest, getuid(), getgid()) < 0)
-                        errExit("chown");
-                if (chmod(dest, S_IRUSR | S_IWUSR) < 0)
-                        errExit("chmod");
         }
 
         // delete the temporary file
diff --git a/src/firejail/fs_hostname.c b/src/firejail/fs_hostname.c
index aa391c0cb..87d3742e4 100644
--- a/src/firejail/fs_hostname.c
+++ b/src/firejail/fs_hostname.c
@@ -40,14 +40,10 @@ void fs_hostname(const char *hostname) {
                         exit(1);
                 }
                 fprintf(fp, "%s\n", hostname);
-                fclose(fp);
-                
                 // mode and owner
-                if (chown(RUN_HOSTNAME_FILE, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_HOSTNAME_FILE, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH ) < 0)
-                        errExit("chmod");
-                
+                SET_PERMS_STREAM(fp, 0, 0, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
+                fclose(fp);
+
                 // bind-mount the file on top of /etc/hostname
                 if (mount(RUN_HOSTNAME_FILE, "/etc/hostname", NULL, MS_BIND|MS_REC, NULL) < 0)
                         errExit("mount bind /etc/hostname");
@@ -88,13 +84,9 @@ void fs_hostname(const char *hostname) {
                                 fprintf(fp2, "%s\n", buf);
                 }
                 fclose(fp1);
-                fclose(fp2);
-                
                 // mode and owner
-                if (chown(RUN_HOSTS_FILE, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_HOSTS_FILE, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH ) < 0)
-                        errExit("chmod");
+                SET_PERMS_STREAM(fp2, 0, 0, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
+                fclose(fp2);
                 
                 // bind-mount the file on top of /etc/hostname
                 if (mount(RUN_HOSTS_FILE, "/etc/hosts", NULL, MS_BIND|MS_REC, NULL) < 0)
@@ -126,13 +118,11 @@ void fs_resolvconf(void) {
                         fprintf(fp, "nameserver %d.%d.%d.%d\n", PRINT_IP(cfg.dns2));
                 if (cfg.dns3)
                         fprintf(fp, "nameserver %d.%d.%d.%d\n", PRINT_IP(cfg.dns3));
-                fclose(fp);
-                
+
                 // mode and owner
-                if (chown(RUN_RESOLVCONF_FILE, 0, 0) < 0)
-                        errExit("chown");
-                if (chmod(RUN_RESOLVCONF_FILE, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH ) < 0)
-                        errExit("chmod");
+                SET_PERMS_STREAM(fp, 0, 0, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
+
+                fclose(fp);
                 
                 // bind-mount the file on top of /etc/hostname
                 if (mount(RUN_RESOLVCONF_FILE, "/etc/resolv.conf", NULL, MS_BIND|MS_REC, NULL) < 0)
diff --git a/src/firejail/fs_logger.c b/src/firejail/fs_logger.c
index 30b0fe438..9f33b36d9 100644
--- a/src/firejail/fs_logger.c
+++ b/src/firejail/fs_logger.c
@@ -97,11 +97,7 @@ void fs_logger_print(void) {
                 perror("fopen");                
                 return;
         }
-        
-        int rv = chown(RUN_FSLOGGER_FILE, getuid(), getgid());
-        (void) rv; // best effort!
-        rv = chmod(RUN_FSLOGGER_FILE, 0644);
-        (void) rv; // best effort!
+        SET_PERMS_STREAM_NOERR(fp, getuid(), getgid(), 0644);
         
         FsMsg *ptr = head;
         while (ptr) {
diff --git a/src/firejail/fs_mkdir.c b/src/firejail/fs_mkdir.c
index 5bc2df2cc..b2a5927e6 100644
--- a/src/firejail/fs_mkdir.c
+++ b/src/firejail/fs_mkdir.c
@@ -119,9 +119,12 @@ void fs_mkfile(const char *name) {
                 if (!fp)
                         fprintf(stderr, "Warning: cannot create %s file\n", expanded);
                 else {
-                        fclose(fp);
-                        int rv = chmod(expanded, 0600);
+                        int fd = fileno(fp);
+                        if (fd == -1)
+                                errExit("fileno");
+                        int rv = fchmod(fd, 0600);
                         (void) rv;
+                        fclose(fp);
                 }
                 exit(0);
         }