fixes

24 files changed, 142 insertions, 88 deletions

diff --git a/src/faudit/dbus.c b/src/faudit/dbus.c
index 4debf2ff6..d92660536 100644
--- a/src/faudit/dbus.c
+++ b/src/faudit/dbus.c
@@ -35,7 +35,7 @@ int check_unix(const char *sockfile) {
         struct sockaddr_un remote;
         memset(&remote, 0, sizeof(struct sockaddr_un));
         remote.sun_family = AF_UNIX;
-        strcpy(remote.sun_path, sockfile);
+        strncpy(remote.sun_path, sockfile, sizeof(remote.sun_path));
         int len = strlen(remote.sun_path) + sizeof(remote.sun_family);
         if (*sockfile == '@')
                 remote.sun_path[0] = '\0';
diff --git a/src/faudit/pid.c b/src/faudit/pid.c
index a0fb1d921..84b23fe0a 100644
--- a/src/faudit/pid.c
+++ b/src/faudit/pid.c
@@ -46,7 +46,6 @@ void pid_test(void) {
                 /* coverity[toctou] */
                 FILE *fp = fopen(fname, "r");
                 if (!fp) {
-//                      fprintf(stderr, "Warning: cannot open %s\n", fname);
                         free(fname);
                         continue;
                 }
@@ -54,7 +53,6 @@ void pid_test(void) {
                 // read file
                 char buf[100];
                 if (fgets(buf, 10, fp) == NULL) {
-//                      fprintf(stderr, "Warning: cannot read %s\n", fname);
                         fclose(fp);
                         free(fname);
                         continue;
diff --git a/src/firecfg/main.c b/src/firecfg/main.c
index d2566ce22..15ee78384 100644
--- a/src/firecfg/main.c
+++ b/src/firecfg/main.c
@@ -342,6 +342,7 @@ static void fix_desktop_files(void) {
                 if (stat(filename, &sb) == -1)
                         errExit("stat");
 
+                /* coverity[toctou] */
                 int fd = open(filename, O_RDONLY);
                 if (fd == -1)
                         errExit("open");
diff --git a/src/firejail/appimage.c b/src/firejail/appimage.c
index 6a9ca1679..0d1f8cb4d 100644
--- a/src/firejail/appimage.c
+++ b/src/firejail/appimage.c
@@ -51,6 +51,7 @@ void appimage_set(const char *appimage) {
                 printf("AppImage ELF size %lu\n", size);
 
         // open appimage file
+        /* coverity[toctou] */
         int ffd = open(appimage, O_RDONLY|O_CLOEXEC);
         if (ffd == -1) {
                 fprintf(stderr, "Error: cannot open AppImage file\n");
@@ -74,6 +75,10 @@ void appimage_set(const char *appimage) {
                 errExit("asprintf");
                 
         int lfd = open(devloop, O_RDONLY);
+        if (lfd == -1) {
+                fprintf(stderr, "Error: cannot open %s\n", devloop);
+                exit(1);
+        }
         if (ioctl(lfd, LOOP_SET_FD, ffd) == -1) {
                 fprintf(stderr, "Error: cannot configure the loopback device\n");
                 exit(1);
@@ -118,7 +123,7 @@ void appimage_set(const char *appimage) {
         EUID_USER();
 
         // set environment
-        if (appimage && setenv("APPIMAGE", appimage, 1) < 0)
+        if (setenv("APPIMAGE", appimage, 1) < 0)
                 errExit("setenv");
         if (mntdir && setenv("APPDIR", mntdir, 1) < 0)
                 errExit("setenv");
@@ -170,8 +175,10 @@ void appimage_clear(void) {
 
         if (devloop) {
                 int lfd = open(devloop, O_RDONLY);
-                rv = ioctl(lfd, LOOP_CLR_FD, 0);
-                (void) rv;
-                close(lfd);
+                if (lfd != -1) {
+                        rv = ioctl(lfd, LOOP_CLR_FD, 0);
+                        (void) rv;
+                        close(lfd);
+                }
         }
 }
diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c
index 963d95bed..974fbb8a3 100644
--- a/src/firejail/checkcfg.c
+++ b/src/firejail/checkcfg.c
@@ -32,6 +32,7 @@ char *netfilter_default = NULL;
 int checkcfg(int val) {
         assert(val < CFG_MAX);
         int line = 0;
+        FILE *fp = NULL;
 
         if (!initialized) {
                 // initialize defaults
@@ -47,7 +48,7 @@ int checkcfg(int val) {
                 if (asprintf(&fname, "%s/firejail.config", SYSCONFDIR) == -1)
                         errExit("asprintf");
 
-                FILE *fp = fopen(fname, "r");
+                fp = fopen(fname, "r");
                 if (!fp) {
 #ifdef HAVE_GLOBALCFG                   
                         fprintf(stderr, "Error: Firejail configuration file %s not found\n", fname);
@@ -285,6 +286,8 @@ int checkcfg(int val) {
         return cfg_val[val];
         
 errout:
+        if (fp)
+                fclose(fp);
         fprintf(stderr, "Error: invalid line %d in firejail configuration file\n", line );
         exit(1);
 }
diff --git a/src/firejail/fs.c b/src/firejail/fs.c
index 6f9b5a60c..9a2f4facc 100644
--- a/src/firejail/fs.c
+++ b/src/firejail/fs.c
@@ -717,6 +717,7 @@ void fs_overlayfs(void) {
                         }
                 }
                 else {
+                        /* coverity[toctou] */
                         if (mkdir(basedir, 0755) != 0) {
                                 fprintf(stderr, "Error: cannot create overlay directory\n");
                                 exit(1);
diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c
index 421df717d..7c56d524e 100644
--- a/src/firejail/fs_bin.c
+++ b/src/firejail/fs_bin.c
@@ -59,6 +59,7 @@ static char *check_dir_or_file(const char *name) {
                 if (stat(fname, &s) == 0 && !S_ISDIR(s.st_mode)) { // do not allow directories
                         // check symlink to firejail executable in /usr/local/bin
                         if (strcmp(paths[i], "/usr/local/bin") == 0 && is_link(fname)) {
+                                /* coverity[toctou] */
                                 char *actual_path = realpath(fname, NULL);
                                 if (actual_path) {
                                         char *ptr = strstr(actual_path, "/firejail");
diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c
index 1f8da398e..0872bf0d0 100644
--- a/src/firejail/fs_home.c
+++ b/src/firejail/fs_home.c
@@ -137,6 +137,7 @@ static int store_asoundrc(void) {
         if (stat(src, &s) == 0) {
                 if (is_link(src)) {
                         // make sure the real path of the file is inside the home directory
+                        /* coverity[toctou] */
                         char* rp = realpath(src, NULL);
                         if (!rp) {
                                 fprintf(stderr, "Error: Cannot access %s\n", src);
diff --git a/src/firejail/fs_hostname.c b/src/firejail/fs_hostname.c
index dcf06fc6f..b2e1b4a99 100644
--- a/src/firejail/fs_hostname.c
+++ b/src/firejail/fs_hostname.c
@@ -52,8 +52,10 @@ void fs_hostname(const char *hostname) {
                         goto errexit;
 
                 FILE *fp2 = fopen(RUN_HOSTS_FILE, "w");
-                if (!fp2)
+                if (!fp2) {
+                        fclose(fp1);
                         goto errexit;
+                }
                 
                 char buf[4096];
                 int done = 0;
diff --git a/src/firejail/fs_mkdir.c b/src/firejail/fs_mkdir.c
index 6bcb3f33e..5b6ceae90 100644
--- a/src/firejail/fs_mkdir.c
+++ b/src/firejail/fs_mkdir.c
@@ -37,6 +37,7 @@ static void mkdir_recursive(char *path) {
         subdir = strtok(path, "/");
         while(subdir) {
                 if (stat(subdir, &s) == -1) {
+                        /* coverity[toctou] */
                         if (mkdir(subdir, 0700) == -1) {
                                 fprintf(stderr, "Warning: cannot create %s directory\n", subdir);
                                 return;
@@ -118,6 +119,7 @@ void fs_mkfile(const char *name) {
                 // drop privileges
                 drop_privs(0);
 
+                /* coverity[toctou] */
                 FILE *fp = fopen(expanded, "w");
                 if (!fp)
                         fprintf(stderr, "Warning: cannot create %s file\n", expanded);
diff --git a/src/firejail/fs_var.c b/src/firejail/fs_var.c
index ca50685ad..2aa4a1b54 100644
--- a/src/firejail/fs_var.c
+++ b/src/firejail/fs_var.c
@@ -128,16 +128,18 @@ void fs_var_log(void) {
                 // create an empty /var/log/wtmp file
                 /* coverity[toctou] */
                 FILE *fp = fopen("/var/log/wtmp", "w");
-                SET_PERMS_STREAM(fp, 0, wtmp_group, S_IRUSR | S_IWRITE | S_IRGRP | S_IWGRP | S_IROTH);
-                if (fp)
+                if (fp) {
+                        SET_PERMS_STREAM(fp, 0, wtmp_group, S_IRUSR | S_IWRITE | S_IRGRP | S_IWGRP | S_IROTH);
                         fclose(fp);
+                }
                 fs_logger("touch /var/log/wtmp");
                         
                 // create an empty /var/log/btmp file
                 fp = fopen("/var/log/btmp", "w");
-                SET_PERMS_STREAM(fp, 0, wtmp_group, S_IRUSR | S_IWRITE | S_IRGRP | S_IWGRP);
-                if (fp)
+                if (fp) {
+                        SET_PERMS_STREAM(fp, 0, wtmp_group, S_IRUSR | S_IWRITE | S_IRGRP | S_IWGRP);
                         fclose(fp);
+                }
                 fs_logger("touch /var/log/btmp");
         }
         else
diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c
index 564dc8290..7b32021be 100644
--- a/src/firejail/fs_whitelist.c
+++ b/src/firejail/fs_whitelist.c
@@ -350,6 +350,8 @@ void fs_whitelist(void) {
                 }
 
                 // replace ~/ or ${HOME} into /home/username
+//              if (new_name)
+//                      free(new_name);
                 new_name = expand_home(entry->data + 10, cfg.homedir);
                 assert(new_name);
                 if (arg_debug)
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index 5444ad9c2..4b4ae1de2 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -259,6 +259,7 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                 drop_privs(0);
 
                 // check access
+                /* coverity[toctou] */
                 if (access(fname1, R_OK) == -1) {
                         fprintf(stderr, "Error: Cannot access %s\n", fname1);
                         exit(1);
@@ -392,6 +393,10 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                 // create a user-owned temporary file in /run/firejail directory
                 char tmp_fname[] = "/run/firejail/tmpget-XXXXXX";
                 int fd = mkstemp(tmp_fname);
+                if (fd == -1) {
+                        fprintf(stderr, "Error: cannot create temporary file %s\n", tmp_fname);
+                        exit(1);
+                }
                 SET_PERMS_FD(fd, getuid(), getgid(), 0600);
                 close(fd);
         
diff --git a/src/firejail/netfilter.c b/src/firejail/netfilter.c
index 1df4b7a0f..0136ab1f8 100644
--- a/src/firejail/netfilter.c
+++ b/src/firejail/netfilter.c
@@ -69,31 +69,33 @@ void netfilter(const char *fname) {
         if (netfilter_default)
                 fname = netfilter_default;
         if (fname) {
-                // buffer the filter
-                struct stat s;
-                if (stat(fname, &s) == -1) {
-                        fprintf(stderr, "Error: cannot find network filter file %s\n", fname);
-                        exit(1);
-                }
-
-                filter = malloc(s.st_size + 1);   // + '\0'
-                if (!filter)
-                        errExit("malloc");
-                memset(filter, 0, s.st_size + 1);
-
-                /* coverity[toctou] */
-                FILE *fp = fopen(fname, "r");
-                if (!fp) {
-                        fprintf(stderr, "Error: cannot open network filter file %s\n", fname);
-                        exit(1);
-                }
-
-                size_t sz = fread(filter, 1, s.st_size, fp);
-                if ((off_t)sz != s.st_size) {
-                        fprintf(stderr, "Error: cannot read network filter file %s\n", fname);
-                        exit(1);
+                assert(fname);
+                
+                // open filter file
+                int fd = open(fname, O_RDONLY);
+                if (fd == -1)
+                        goto errexit;
+                int size = lseek(fd, 0, SEEK_END);
+                if (size == -1)
+                        goto errexit;
+                if (lseek(fd, 0 , SEEK_SET) == -1)
+                        goto errexit;
+                
+                // read filter
+                filter = malloc(size + 1);        // + '\0'
+                if (filter == NULL)
+                        goto errexit;
+                memset(&filter[0], 0, sizeof(filter));
+                int rd = 0;
+                while (rd < size) {
+                        int rv = read(fd, (unsigned char *) filter + rd, size - rd);
+                        if (rv == -1)
+                                goto errexit;
+                        rd += rv;
                 }
-                fclose(fp);
+                
+                // close file
+                close(fd);
                 allocated = 1;
         }
 
@@ -178,6 +180,11 @@ doexit:
 
         if (allocated)
                 free(filter);
+        return;
+
+errexit:
+        fprintf(stderr, "Error: cannot read network filter %s\n", fname);
+        exit(1);
 }
 
 void netfilter6(const char *fname) {
@@ -186,38 +193,38 @@ void netfilter6(const char *fname) {
                 
         char *filter;
 
-        // buffer the filter
-        struct stat s;
-        if (stat(fname, &s) == -1) {
-                fprintf(stderr, "Error: cannot find network filter file %s\n", fname);
-                exit(1);
-        }
-
-        filter = malloc(s.st_size + 1);   // + '\0'
-        if (!filter)
-                errExit("malloc");
-        memset(filter, 0, s.st_size + 1);
-
-        /* coverity[toctou] */
-        FILE *fp = fopen(fname, "r");
-        if (!fp) {
-                fprintf(stderr, "Error: cannot open network filter file %s\n", fname);
-                exit(1);
-        }
-
-        size_t sz = fread(filter, 1, s.st_size, fp);
-        if ((off_t)sz != s.st_size) {
-                fprintf(stderr, "Error: cannot read network filter file %s\n", fname);
-                exit(1);
+        // open filter file
+        int fd = open(fname, O_RDONLY);
+        if (fd == -1)
+                goto errexit;
+        int size = lseek(fd, 0, SEEK_END);
+        if (size == -1)
+                goto errexit;
+        if (lseek(fd, 0 , SEEK_SET) == -1)
+                goto errexit;
+        
+        // read filter
+        filter = malloc(size + 1);        // + '\0'
+        if (filter == NULL)
+                goto errexit;
+        memset(&filter[0], 0, sizeof(filter));
+        int rd = 0;
+        while (rd < size) {
+                int rv = read(fd, (unsigned char *) filter + rd, size - rd);
+                if (rv == -1)
+                        goto errexit;
+                rd += rv;
         }
-        fclose(fp);
+        
+        // close file
+        close(fd);
 
         // temporarily mount a tempfs on top of /tmp directory
         if (mount("tmpfs", "/tmp", "tmpfs", MS_NOSUID | MS_STRICTATIME | MS_REC,  "mode=755,gid=0") < 0)
                 errExit("mounting /tmp");
 
         // create the filter file
-        fp = fopen("/tmp/netfilter6", "w");
+        FILE *fp = fopen("/tmp/netfilter6", "w");
         if (!fp) {
                 fprintf(stderr, "Error: cannot open /tmp/netfilter6 file\n");
                 exit(1);
@@ -228,6 +235,7 @@ void netfilter6(const char *fname) {
         // find iptables command
         char *ip6tables = NULL;
         char *ip6tables_restore = NULL;
+        struct stat s;
         if (stat("/sbin/ip6tables", &s) == 0) {
                 ip6tables = "/sbin/ip6tables";
                 ip6tables_restore = "/sbin/ip6tables-restore";
@@ -284,4 +292,9 @@ doexit:
         // unmount /tmp
         umount("/tmp");
         free(filter);
+        return;
+        
+errexit:
+        fprintf(stderr, "Error: cannot read network filter %s\n", fname);
+        exit(1);
 }
diff --git a/src/firejail/no_sandbox.c b/src/firejail/no_sandbox.c
index aae490c34..8af555ea2 100644
--- a/src/firejail/no_sandbox.c
+++ b/src/firejail/no_sandbox.c
@@ -232,7 +232,7 @@ void run_no_sandbox(int argc, char **argv) {
         // use $SHELL to get shell used in sandbox
         if (!arg_shell_none && !cfg.shell) {
                 char *shell =  getenv("SHELL");
-                if (access(shell, R_OK) == 0)
+                if (shell && access(shell, R_OK) == 0)
                         cfg.shell = shell;
         }
         // guess shell otherwise
diff --git a/src/firejail/profile.c b/src/firejail/profile.c
index 694509511..9acb1b813 100644
--- a/src/firejail/profile.c
+++ b/src/firejail/profile.c
@@ -990,6 +990,7 @@ void profile_read(const char *fname) {
                 // process quiet
                 if (strcmp(ptr, "quiet") == 0) {
                         arg_quiet = 1;
+                        free(ptr);
                         continue;
                 }
                 if (!msg_printed) {
diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c
index 6ec590eaa..f890dd534 100644
--- a/src/firejail/pulseaudio.c
+++ b/src/firejail/pulseaudio.c
@@ -137,6 +137,7 @@ void pulseaudio_init(void) {
         if (asprintf(&dir1, "%s/.config/pulse", cfg.homedir) == -1)
                 errExit("asprintf");
         if (stat(dir1, &s) == -1) {
+                /* coverity[toctou] */
                 int rv = mkdir(dir1, 0700);
                 if (rv == 0) {
                         if (set_perms(dir1, getuid(), getgid(), 0700))
diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c
index 8aa2fe53f..a4dce405d 100644
--- a/src/firejail/run_symlink.c
+++ b/src/firejail/run_symlink.c
@@ -59,6 +59,7 @@ void run_symlink(int argc, char **argv) {
 
                 struct stat s;
                 if (stat(name, &s) == 0) {
+                        /* coverity[toctou] */
                         char* rp = realpath(name, NULL);
                         if (!rp)
                                 errExit("realpath");
diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c
index 430ffb86e..dbfdd445a 100644
--- a/src/firejail/sbox.c
+++ b/src/firejail/sbox.c
@@ -150,6 +150,7 @@ int sbox_run(unsigned filter, int num, ...) {
                         }
                         else // the user could run the sandbox without /dev/null
                                 close(STDIN_FILENO);
+                        close(fd);
                 }
                 umask(027);     
 
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 03f52fabb..c3e00a110 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -683,11 +683,12 @@ int remove_directory(const char *path) {
 void flush_stdin(void) {
         if (isatty(STDIN_FILENO)) {
                 int cnt = 0;
-                ioctl(STDIN_FILENO, FIONREAD, &cnt);
-                if (cnt) {
+                int rv = ioctl(STDIN_FILENO, FIONREAD, &cnt);
+                if (rv == 0 && cnt) {
                         if (!arg_quiet)
                                 printf("Warning: removing %d bytes from stdin\n", cnt);
-                        ioctl(STDIN_FILENO, TCFLSH, TCIFLUSH);
+                        rv = ioctl(STDIN_FILENO, TCFLSH, TCIFLUSH);
+                        (void) rv;
                 }
         }
 }
@@ -700,6 +701,7 @@ void create_empty_dir_as_root(const char *dir, mode_t mode) {
         if (stat(dir, &s)) {
                 if (arg_debug)
                         printf("Creating empty %s directory\n", dir);
+                /* coverity[toctou] */
                 if (mkdir(dir, mode) == -1)
                         errExit("mkdir");
                 if (set_perms(dir, 0, 0, mode))
@@ -717,6 +719,7 @@ void create_empty_file_as_root(const char *fname, mode_t mode) {
                 if (arg_debug)
                         printf("Creating empty %s file\n", fname);
 
+                /* coverity[toctou] */
                 FILE *fp = fopen(fname, "w");
                 if (!fp)
                         errExit("fopen");
diff --git a/src/firejail/x11.c b/src/firejail/x11.c
index 9da6d3e30..807f2d5f0 100644
--- a/src/firejail/x11.c
+++ b/src/firejail/x11.c
@@ -151,6 +151,7 @@ void fs_x11(void) {
         fs_logger("tmpfs /tmp/.X11-unix");
 
         // create an empty file
+        /* coverity[toctou] */
         FILE *fp = fopen(x11file, "w");
         if (!fp) {
                 fprintf(stderr, "Error: cannot create empty file in x11 directory\n");
diff --git a/src/fseccomp/seccomp_print.c b/src/fseccomp/seccomp_print.c
index 7dc983b12..af240307c 100644
--- a/src/fseccomp/seccomp_print.c
+++ b/src/fseccomp/seccomp_print.c
@@ -26,35 +26,41 @@ static int filter_cnt = 0;
 
 static void load_seccomp(const char *fname) {
         assert(fname);
+        
+        // open filter file
+        int fd = open(fname, O_RDONLY);
+        if (fd == -1)
+                goto errexit;
 
-        // check file
-        struct stat s;
-        if (stat(fname, &s) == -1) {
-                fprintf(stderr, "Error fseccomp: cannot read protocol filter file\n");
-                exit(1);
-        }
-        int size = s.st_size;
+        // calculate the number of entries
+        int size = lseek(fd, 0, SEEK_END);
+        if (size == -1)
+                goto errexit;
+        if (lseek(fd, 0 , SEEK_SET) == -1)
+                goto errexit;
         unsigned short entries = (unsigned short) size / (unsigned short) sizeof(struct sock_filter);
         filter_cnt = entries;
-//printf("size %d, entries %d\n", s.st_size, entries);
-
-        filter = malloc(sizeof(struct sock_filter) * entries);
-        if (!filter)
-                errExit("malloc");
-                
+        
         // read filter
-        memset(filter, 0, sizeof(struct sock_filter) * entries);
-        int src = open(fname, O_RDONLY);
+        filter = malloc(size);
+        if (filter == NULL)
+                goto errexit;
+        memset(&filter[0], 0, sizeof(filter));
         int rd = 0;
         while (rd < size) {
-                int rv = read(src, (unsigned char *) filter + rd, size - rd);
-                if (rv == -1) {
-                        fprintf(stderr, "Error fseccomp: cannot read %s file\n", fname);
-                        exit(1);
-                }
+                int rv = read(fd, (unsigned char *) filter + rd, size - rd);
+                if (rv == -1)
+                        goto errexit;
                 rd += rv;
         }
-        close(src);
+        
+        // close file
+        close(fd);
+        return;
+
+errexit:
+        fprintf(stderr, "Error fseccomp: cannot read %s\n", fname);
+        exit(1);
 }
 
 // debug filter
diff --git a/src/lib/common.c b/src/lib/common.c
index add4ff087..3f66fa72a 100644
--- a/src/lib/common.c
+++ b/src/lib/common.c
@@ -203,6 +203,8 @@ char *pid_proc_cmdline(const pid_t pid) {
 int pid_proc_cmdline_x11_xpra_xephyr(const pid_t pid) {
         // if comm is not firejail return 0
         char *comm = pid_proc_comm(pid);
+        if (comm == NULL)
+                return 0;
         if (strcmp(comm, "firejail") != 0) {
                 free(comm);
                 return 0;
diff --git a/src/libtracelog/libtracelog.c b/src/libtracelog/libtracelog.c
index ca496d41c..90fe726de 100644
--- a/src/libtracelog/libtracelog.c
+++ b/src/libtracelog/libtracelog.c
@@ -191,7 +191,7 @@ static void load_blacklist(void) {
                         char *ptr = strchr(buf, '\n');
                         if (ptr)
                                 *ptr = '\0';
-                        if (sandbox_name_str == NULL);
+                        if (sandbox_name_str == NULL)
                                 sandbox_name_str = strdup(buf + 14);
                 }
                 else if (strncmp(buf, "blacklist ", 10) == 0) {