10 files changed, 133 insertions, 208 deletions

diff --git a/src/faudit/syscall.c b/src/faudit/syscall.c
index 3c87305df..4cd2526ba 100644
--- a/src/faudit/syscall.c
+++ b/src/faudit/syscall.c
@@ -35,7 +35,8 @@ void syscall_helper(int argc, char **argv) {
         (void) argc;
         
         if (strcmp(argv[2], "mount") == 0) {
-                mount(NULL, NULL, NULL, 0, NULL);
+                int rv = mount(NULL, NULL, NULL, 0, NULL);
+                (void) rv;
                 printf("\nUGLY: mount syscall permitted.\n");
         }
         else if (strcmp(argv[2], "umount2") == 0) {
diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c
index 974fbb8a3..6565f488a 100644
--- a/src/firejail/checkcfg.c
+++ b/src/firejail/checkcfg.c
@@ -33,6 +33,7 @@ int checkcfg(int val) {
         assert(val < CFG_MAX);
         int line = 0;
         FILE *fp = NULL;
+        char *ptr;
 
         if (!initialized) {
                 // initialize defaults
@@ -76,7 +77,7 @@ int checkcfg(int val) {
                                 continue;
 
                         // parse line                           
-                        char *ptr = line_remove_spaces(buf);
+                        ptr = line_remove_spaces(buf);
                         if (!ptr)
                                 continue;
                         
@@ -286,8 +287,10 @@ int checkcfg(int val) {
         return cfg_val[val];
         
 errout:
-        if (fp)
-                fclose(fp);
+        assert(ptr);
+        free(ptr);
+        assert(fp);
+        fclose(fp);
         fprintf(stderr, "Error: invalid line %d in firejail configuration file\n", line );
         exit(1);
 }
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 4ae3cfd9f..61de17bf8 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -41,7 +41,6 @@
 #define RUN_CPU_CFG     "/run/firejail/mnt/cpu"
 #define RUN_GROUPS_CFG  "/run/firejail/mnt/groups"
 #define RUN_PROTOCOL_CFG        "/run/firejail/mnt/protocol"
-#define RUN_CP_COMMAND  "/run/firejail/mnt/cp"
 #define RUN_HOME_DIR    "/run/firejail/mnt/home"
 #define RUN_ETC_DIR     "/run/firejail/mnt/etc"
 #define RUN_BIN_DIR     "/run/firejail/mnt/bin"
@@ -463,6 +462,7 @@ void create_empty_dir_as_root(const char *dir, mode_t mode);
 void create_empty_file_as_root(const char *dir, mode_t mode);
 int set_perms(const char *fname, uid_t uid, gid_t gid, mode_t mode);
 void mkdir_attr(const char *fname, mode_t mode, uid_t uid, gid_t gid);
+char *read_text_file_or_exit(const char *fname);
 
 // fs_var.c
 void fs_var_log(void);  // mounting /var/log
@@ -679,6 +679,8 @@ void build_cmdline(char **command_line, char **window_title, int argc, char **ar
 #define PATH_FIREMON (PREFIX "/bin/firemon")
 #define PATH_FSECCOMP (LIBDIR "/firejail/fseccomp")
 #define PATH_FCOPY (LIBDIR "/firejail/fcopy")
+#define SBOX_STDIN_FILE "/run/firejail/mnt/sbox_stdin"
+
 // bitmapped filters for sbox_run
 #define SBOX_ROOT (1 << 0)                      // run the sandbox as root
 #define SBOX_USER (1 << 1)                      // run the sandbox as a regular user
@@ -686,6 +688,7 @@ void build_cmdline(char **command_line, char **window_title, int argc, char **ar
 #define SBOX_CAPS_NONE (1 << 3)         // drop all capabilities
 #define SBOX_CAPS_NETWORK (1 << 4)      // caps filter for programs running network programs
 #define SBOX_ALLOW_STDIN (1 << 5)               // don't close stdin
+#define SBOX_STDIN_FROM_FILE (1 << 6)   // open file and redirect it to stdin
 
 // run sbox
 int sbox_run(unsigned filter, int num, ...);
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index 4b4ae1de2..77eb35f97 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -259,11 +259,11 @@ 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);
                 }
+                /* coverity[toctou] */
                 char *rp = realpath(fname1, NULL);
                 if (!rp) {
                         fprintf(stderr, "Error: Cannot access %s\n", fname1);
@@ -316,9 +316,11 @@ 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);
-                SET_PERMS_FD(fd, getuid(), getgid(), 0600);
-                close(fd);
-        
+                if (fd != -1) {
+                        SET_PERMS_FD(fd, getuid(), getgid(), 0600);
+                        close(fd);
+                }
+                
                 // copy the source file into the temporary file - we need to chroot
                 pid_t child = fork();
                 if (child < 0)
diff --git a/src/firejail/netfilter.c b/src/firejail/netfilter.c
index 0136ab1f8..ef4915f15 100644
--- a/src/firejail/netfilter.c
+++ b/src/firejail/netfilter.c
@@ -61,57 +61,6 @@ void check_netfilter_file(const char *fname) {
 
 
 void netfilter(const char *fname) {
-        // default filter
-        char *filter = client_filter;
-
-        // custom filter
-        int allocated = 0;
-        if (netfilter_default)
-                fname = netfilter_default;
-        if (fname) {
-                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;
-                }
-                
-                // close file
-                close(fd);
-                allocated = 1;
-        }
-
-        // 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
-        FILE *fp = fopen("/tmp/netfilter", "w");
-        if (!fp) {
-                fprintf(stderr, "Error: cannot open /tmp/netfilter file\n");
-                exit(1);
-        }
-        fprintf(fp, "%s\n", filter);
-        fclose(fp);
-
         // find iptables command
         struct stat s;
         char *iptables = NULL;
@@ -125,113 +74,49 @@ void netfilter(const char *fname) {
                 iptables_restore = "/usr/sbin/iptables-restore";
         }
         if (iptables == NULL || iptables_restore == NULL) {
-                fprintf(stderr, "Error: iptables command not found\n");
-                goto doexit;
+                fprintf(stderr, "Error: iptables command not found, netfilter not configured\n");
+                return;
         }
 
-        // push filter
-        pid_t child = fork();
-        if (child < 0)
-                errExit("fork");
-        if (child == 0) {
-                if (arg_debug)
-                        printf("Installing network filter:\n%s\n", filter);
-
-                int fd;
-                if((fd = open("/tmp/netfilter", O_RDONLY)) == -1) {
-                        fprintf(stderr,"Error: cannot open /tmp/netfilter\n");
-                        exit(1);
-                }
-                dup2(fd,STDIN_FILENO);
-
-                // wipe out environment variables
-                clearenv();     
-                execl(iptables_restore, iptables_restore, NULL);
-                perror("execl");
-                _exit(1);
+        // read filter
+        char *filter = client_filter;
+        int allocated = 0;
+        if (netfilter_default)
+                fname = netfilter_default;
+        if (fname) {
+                filter = read_text_file_or_exit(fname);
+                allocated = 1;
         }
-        // wait for the child to finish
-        waitpid(child, NULL, 0);
 
-        // debug
-        if (arg_debug) {
-                child = fork();
-                if (child < 0)
-                        errExit("fork");
-                if (child == 0) {
-                        // elevate privileges in order to get grsecurity working
-                        if (setreuid(0, 0))
-                                errExit("setreuid");
-                        if (setregid(0, 0))
-                                errExit("setregid");
-                        environ = NULL;
-                        assert(getenv("LD_PRELOAD") == NULL);   
-                        execl(iptables, iptables, "-vL", NULL);
-                        perror("execl");
-                        _exit(1);
-                }
-                // wait for the child to finish
-                waitpid(child, NULL, 0);
+        // create the filter file
+        FILE *fp = fopen(SBOX_STDIN_FILE, "w");
+        if (!fp) {
+                fprintf(stderr, "Error: cannot open %s\n", SBOX_STDIN_FILE);
+                exit(1);
         }
+        fprintf(fp, "%s\n", filter);
+        fclose(fp);
+
 
-doexit:
-        // unmount /tmp
-        umount("/tmp");
+        // push filter
+        if (arg_debug)
+                printf("Installing network filter:\n%s\n", filter);
+        sbox_run(SBOX_ROOT | SBOX_CAPS_NETWORK | SBOX_SECCOMP | SBOX_STDIN_FROM_FILE, 1, iptables_restore);
+        unlink(SBOX_STDIN_FILE);
+
+        // debug
+        if (arg_debug) 
+                sbox_run(SBOX_ROOT | SBOX_CAPS_NETWORK | SBOX_SECCOMP, 2, iptables, "-vL");
 
         if (allocated)
                 free(filter);
         return;
-
-errexit:
-        fprintf(stderr, "Error: cannot read network filter %s\n", fname);
-        exit(1);
 }
 
 void netfilter6(const char *fname) {
         if (fname == NULL)
                 return;
                 
-        char *filter;
-
-        // 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;
-        }
-        
-        // 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
-        FILE *fp = fopen("/tmp/netfilter6", "w");
-        if (!fp) {
-                fprintf(stderr, "Error: cannot open /tmp/netfilter6 file\n");
-                exit(1);
-        }
-        fprintf(fp, "%s\n", filter);
-        fclose(fp);
-
         // find iptables command
         char *ip6tables = NULL;
         char *ip6tables_restore = NULL;
@@ -245,56 +130,30 @@ void netfilter6(const char *fname) {
                 ip6tables_restore = "/usr/sbin/ip6tables-restore";
         }
         if (ip6tables == NULL || ip6tables_restore == NULL) {
-                fprintf(stderr, "Error: ip6tables command not found\n");
-                goto doexit;
+                fprintf(stderr, "Error: ip6tables command not found, netfilter6 not configured\n");
+                return;
         }
 
-        // push filter
-        pid_t child = fork();
-        if (child < 0)
-                errExit("fork");
-        if (child == 0) {
-                if (arg_debug)
-                        printf("Installing network filter:\n%s\n", filter);
-
-                int fd;
-                if((fd = open("/tmp/netfilter6", O_RDONLY)) == -1) {
-                        fprintf(stderr,"Error: cannot open /tmp/netfilter6\n");
-                        exit(1);
-                }
-                dup2(fd,STDIN_FILENO);
-
-                // wipe out environment variables
-                clearenv();     
-                execl(ip6tables_restore, ip6tables_restore, NULL);
-                perror("execl");
-                _exit(1);
+        // create the filter file
+        char *filter = read_text_file_or_exit(fname);
+        FILE *fp = fopen(SBOX_STDIN_FILE, "w");
+        if (!fp) {
+                fprintf(stderr, "Error: cannot open /tmp/netfilter6 file\n");
+                exit(1);
         }
-        // wait for the child to finish
-        waitpid(child, NULL, 0);
+        fprintf(fp, "%s\n", filter);
+        fclose(fp);
 
+        // push filter
+        if (arg_debug)
+                printf("Installing network filter:\n%s\n", filter);
+        sbox_run(SBOX_ROOT | SBOX_CAPS_NETWORK | SBOX_SECCOMP | SBOX_STDIN_FROM_FILE, 1, ip6tables_restore);
+        unlink(SBOX_STDIN_FILE);
+        
         // debug
-        if (arg_debug) {
-                child = fork();
-                if (child < 0)
-                        errExit("fork");
-                if (child == 0) {
-                        clearenv();     
-                        execl(ip6tables, ip6tables, "-vL", NULL);
-                        perror("execl");
-                        _exit(1);
-                }
-                // wait for the child to finish
-                waitpid(child, NULL, 0);
-        }
+        if (arg_debug)
+                sbox_run(SBOX_ROOT | SBOX_CAPS_NETWORK | SBOX_SECCOMP, 2, ip6tables, "-vL");
 
-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/sbox.c b/src/firejail/sbox.c
index dbfdd445a..f28bbaf1a 100644
--- a/src/firejail/sbox.c
+++ b/src/firejail/sbox.c
@@ -138,20 +138,34 @@ int sbox_run(unsigned filter, int num, ...) {
         if (child == 0) {
                 // clean the new process
                 clearenv();
-                int max = 20; // getdtablesize() is overkill for a firejail process
-                for (i = 3; i < max; i++)
-                        close(i); // close open files
-                if ((filter & SBOX_ALLOW_STDIN) == 0) {
-                        int fd = open("/dev/null",O_RDWR, 0);
-                        if (fd != -1) {
-                                dup2 (fd, STDIN_FILENO);
-                                if (fd > 2)
-                                        close (fd);
+                
+                if (filter & SBOX_STDIN_FROM_FILE) {
+                        int fd;
+                        if((fd = open(SBOX_STDIN_FILE, O_RDONLY)) == -1) {
+                                fprintf(stderr,"Error: cannot open /tmp/netfilter\n");
+                                exit(1);
                         }
+                        dup2(fd,STDIN_FILENO);
+                }
+                else if ((filter & SBOX_ALLOW_STDIN) == 0) {
+                        int fd = open("/dev/null",O_RDWR, 0);
+                        if (fd != -1)
+                                dup2(fd, STDIN_FILENO);
                         else // the user could run the sandbox without /dev/null
                                 close(STDIN_FILENO);
-                        close(fd);
                 }
+                
+                // close all other file descriptors
+                int max = 20; // getdtablesize() is overkill for a firejail process
+                for (i = 3; i < max; i++)
+                        close(i); // close open files
+
+                if (arg_debug) {
+                        printf("sbox file descriptors:\n");
+                        int rv = system("ls -l /proc/self/fd");
+                        (void) rv;      
+                }
+
                 umask(027);     
 
                 // apply filters
diff --git a/src/firejail/seccomp.c b/src/firejail/seccomp.c
index dd133b2ba..cdbbe4fdd 100644
--- a/src/firejail/seccomp.c
+++ b/src/firejail/seccomp.c
@@ -72,7 +72,7 @@ int seccomp_load(const char *fname) {
         struct sock_filter *filter = malloc(size);
         if (filter == NULL)
                 goto errexit;
-        memset(&filter[0], 0, sizeof(filter));
+        memset(filter, 0, size);
         int rd = 0;
         while (rd < size) {
                 int rv = read(fd, (unsigned char *) filter + rd, size - rd);
diff --git a/src/firejail/util.c b/src/firejail/util.c
index c3e00a110..75f2acdb9 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -777,3 +777,45 @@ void mkdir_attr(const char *fname, mode_t mode, uid_t uid, gid_t gid) {
 
         ASSERT_PERMS(fname, uid, gid, mode);
 }
+
+char *read_text_file_or_exit(const char *fname) {
+        assert(fname);
+        
+        // open file
+        int fd = open(fname, O_RDONLY);
+        if (fd == -1) {
+                fprintf(stderr, "Error: cannot read %s\n", fname);
+                exit(1);
+        }
+
+        int size = lseek(fd, 0, SEEK_END);
+        if (size == -1)
+                goto errexit;
+        if (lseek(fd, 0 , SEEK_SET) == -1)
+                goto errexit;
+        
+        // allocate memory
+        char *data = malloc(size + 1);    // + '\0'
+        if (data == NULL)
+                goto errexit;
+        memset(data, 0, size + 1);
+
+        // read file
+        int rd = 0;
+        while (rd < size) {
+                int rv = read(fd, (unsigned char *) data + rd, size - rd);
+                if (rv == -1) {
+                        goto errexit;
+                }
+                rd += rv;
+        }
+        
+        // close file
+        close(fd);
+        return data;
+        
+errexit:
+        close(fd);
+        fprintf(stderr, "Error: cannot read %s\n", fname);
+        exit(1);
+}
\ No newline at end of file
diff --git a/src/firejail/x11.c b/src/firejail/x11.c
index 807f2d5f0..e67260490 100644
--- a/src/firejail/x11.c
+++ b/src/firejail/x11.c
@@ -252,7 +252,7 @@ void x11_start_xephyr(int argc, char **argv) {
                 }
 
                 for (i = 0; i < (int) strlen(xephyr_extra_params)-1; i++) {
-                        if (pos >= (sizeof(server_argv)/sizeof(*server_argv))) {
+                        if (pos >= (sizeof(server_argv)/sizeof(*server_argv)) - 2) {
                                 fprintf(stderr, "Error: arg count limit exceeded while parsing xephyr_extra_params\n");
                                 exit(1);
                         }
@@ -716,6 +716,7 @@ void x11_xorg(void) {
         }
         if (set_perms(RUN_XAUTHORITY_SEC_FILE, getuid(), getgid(), 0600))
                 errExit("set_perms");
+        /* coverity[toctou] */
         unlink(tmpfname);
         
         // mount
diff --git a/src/fseccomp/seccomp_print.c b/src/fseccomp/seccomp_print.c
index af240307c..e22c682dc 100644
--- a/src/fseccomp/seccomp_print.c
+++ b/src/fseccomp/seccomp_print.c
@@ -45,7 +45,7 @@ static void load_seccomp(const char *fname) {
         filter = malloc(size);
         if (filter == NULL)
                 goto errexit;
-        memset(&filter[0], 0, sizeof(filter));
+        memset(filter, 0, size);
         int rd = 0;
         while (rd < size) {
                 int rv = read(fd, (unsigned char *) filter + rd, size - rd);