11 files changed, 105 insertions, 135 deletions

diff --git a/RELNOTES b/RELNOTES
index 44d313999..3a9ccaa4b 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -1,8 +1,11 @@
 firejail (0.9.45) baseline; urgency=low
   * development version, work in progress
   * security: overwrite /etc/resolv.conf found by Martin Carpenter
+  * secuirty: TOCTOU exploit for --get and --put found by Daniel Hodson
+  * security: invalid environment exploit found by Martin Carpener
+  * security: split most of networking code in a separate executable
+  * security: split seccomp filter code code in a separate executable
   * feature: allow root user access to /dev/shm (--noblacklist=/dev/shm)
-  * feature: split most of networking code in a separate executable
   * new profiles: xiphos, Tor Browser Bundle, display (imagemagik), Wire
   * bugfixes
  -- netblue30 <netblue30@yahoo.com>  Sun, 23 Oct 2016 08:00:00 -0500
diff --git a/src/firejail/bandwidth.c b/src/firejail/bandwidth.c
index 4a1df9c67..ab9714afe 100644
--- a/src/firejail/bandwidth.c
+++ b/src/firejail/bandwidth.c
@@ -462,6 +462,7 @@ void bandwidth_pid(pid_t pid, const char *command, const char *dev, int down, in
         arg[1] = "-c";
         arg[2] = cmd;
         arg[3] = NULL;
+        assert(getenv("LD_PRELOAD") == NULL);   
         execvp(arg[0], arg);
         
         // it will never get here
diff --git a/src/firejail/fs_bin.c b/src/firejail/fs_bin.c
index f59944735..d6fee0608 100644
--- a/src/firejail/fs_bin.c
+++ b/src/firejail/fs_bin.c
@@ -191,6 +191,7 @@ static void duplicate(char *fname) {
                                 char *f;
                                 if (asprintf(&f, "%s/%s", RUN_BIN_DIR, fname) == -1)
                                         errExit("asprintf");
+                                assert(getenv("LD_PRELOAD") == NULL);   
                                 execlp(RUN_CP_COMMAND, RUN_CP_COMMAND, "-a", actual_path, f, NULL);
                                 perror("execlp");
                                 _exit(1);
diff --git a/src/firejail/fs_etc.c b/src/firejail/fs_etc.c
index b86d5eb74..4f3417236 100644
--- a/src/firejail/fs_etc.c
+++ b/src/firejail/fs_etc.c
@@ -105,6 +105,7 @@ static void duplicate(char *fname) {
                 char *f;
                 if (asprintf(&f, "/etc/%s", fname) == -1)
                         errExit("asprintf");
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execlp(RUN_CP_COMMAND, RUN_CP_COMMAND, "-a", "--parents", f, RUN_MNT_DIR, NULL);
                 perror("execlp");
                 _exit(1);
diff --git a/src/firejail/ls.c b/src/firejail/ls.c
index dba82be0b..7c5585324 100644
--- a/src/firejail/ls.c
+++ b/src/firejail/ls.c
@@ -324,22 +324,24 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
         
         // get file from sandbox and store it in the current directory
         else if (op == SANDBOX_FS_GET) {
-                // check source file (sandbox)
-                char *src_fname;
-                if (asprintf(&src_fname, "%s%s", rootdir, fname1) == -1)
-                        errExit("asprintf");
-                EUID_ROOT();
-                struct stat s;
-                if (stat(src_fname, &s) == -1) {
-                        fprintf(stderr, "Error: Cannot access %s\n", fname1);
-                        exit(1);
-                }
-                if (is_dir(src_fname)) {
-                        fprintf(stderr, "Error: source file name is a directory\n");
+                char *src_fname =fname1;
+                char *dest_fname = strrchr(fname1, '/');
+                if (!dest_fname || *(++dest_fname) == '\0') {
+                        fprintf(stderr, "Error: invalid file name %s\n", fname1);
                         exit(1);
                 }
-                
-                // try to open the source file - we need to chroot
+
+                EUID_ROOT();
+                if (arg_debug)
+                        printf("copy %s to %s\n", src_fname, dest_fname);
+
+                // 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);
+        
+                // copy the source file into the temporary file - we need to chroot
                 pid_t child = fork();
                 if (child < 0)
                         errExit("fork");
@@ -353,11 +355,9 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                         // drop privileges
                         drop_privs(0);
                         
-                        // try to read the file
-                        if (access(fname1, R_OK) == -1) {
-                                fprintf(stderr, "Error: Cannot read %s\n", fname1);
-                                exit(1);
-                        }
+                        // copy the file
+                        if (copy_file(src_fname, tmp_fname, getuid(), getgid(), 0600))
+                                _exit(1);
                         _exit(0);
                 }
 
@@ -365,74 +365,54 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                 int status = 0;
                 waitpid(child, &status, 0);
                 if (WIFEXITED(status) && WEXITSTATUS(status) == 0);
-                else
-                        exit(1);
-                EUID_USER();
-                
-                // check destination file (host)
-                char *dest_fname = strrchr(fname1, '/');
-                if (!dest_fname || *(++dest_fname) == '\0') {
-                        fprintf(stderr, "Error: invalid file name %s\n", fname1);
+                else {
+                        unlink(tmp_fname);
                         exit(1);
                 }
                 
-                if (access(dest_fname, F_OK) == -1) {
-                        // try to create the file as a regular user
-                        pid_t child = fork();
-                        if (child < 0)
-                                errExit("fork");
-                        if (child == 0) {
-                                // drop privileges
-                                drop_privs(0);
-                                
-                                FILE *fp = fopen(dest_fname, "w");
-                                if (!fp) {
-                                        fprintf(stderr, "Error: cannot create %s\n", dest_fname);
-                                        exit(1);
-                                }
-                                fclose(fp);
-                                _exit(0);
-                        }
-
-                        // wait for the child to finish
-                        int status = 0;
-                        waitpid(child, &status, 0);
-                        if (WIFEXITED(status) && WEXITSTATUS(status) == 0);
-                        else
-                                exit(1);
+                // copy the temporary file into the destionation file
+                child = fork();
+                if (child < 0)
+                        errExit("fork");
+                if (child == 0) {
+                        // drop privileges
+                        drop_privs(0);
+                        
+                        // copy the file
+                        if (copy_file(tmp_fname, dest_fname, getuid(), getgid(), 0600))
+                                _exit(1);
+                        _exit(0);
                 }
+
+                // wait for the child to finish
+                status = 0;
+                waitpid(child, &status, 0);
+                if (WIFEXITED(status) && WEXITSTATUS(status) == 0);
                 else {
-                        if (access(dest_fname, W_OK) == -1) {
-                                fprintf(stderr, "Error: cannot write %s\n", dest_fname);
-                                exit(1);
-                        }
+                        unlink(tmp_fname);
+                        exit(1);
                 }
                 
-                // copy file
-                if (arg_debug)
-                        printf("copy %s to %s\n", src_fname, dest_fname);
-                EUID_ROOT();
-                if (copy_file(src_fname, dest_fname, getuid(), getgid(), 0644))
-                        fprintf(stderr, "Error: transfer failed\n");
-                else
-                        printf("Transfer complete\n");
+                // remove the temporary file
+                unlink(tmp_fname);
                 EUID_USER();
         }
         // get file from host and store it in the sandbox
         else if (op == SANDBOX_FS_PUT && path2) {
-                // verify the source file
-                const char *src_fname = path1;
-                struct stat s;
-                if (stat(src_fname, &s) == -1) {
-                        fprintf(stderr, "Error: Cannot access %s\n", fname1);
-                        exit(1);
-                }
-                if (is_dir(src_fname)) {
-                        fprintf(stderr, "Error: source file name is a directory\n");
-                        exit(1);
-                }
-                
-                // try to open the source file 
+                char *src_fname =fname1;
+                char *dest_fname = fname2;
+
+                EUID_ROOT();
+                if (arg_debug)
+                        printf("copy %s to %s\n", src_fname, dest_fname);
+
+                // 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);
+        
+                // copy the source file into the temporary file - we need to chroot
                 pid_t child = fork();
                 if (child < 0)
                         errExit("fork");
@@ -440,11 +420,9 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                         // drop privileges
                         drop_privs(0);
                         
-                        // try to read the file
-                        if (access(src_fname, R_OK) == -1) {
-                                fprintf(stderr, "Error: Cannot read %s\n", src_fname);
-                                exit(1);
-                        }
+                        // copy the file
+                        if (copy_file(src_fname, tmp_fname, getuid(), getgid(), 0600))
+                                _exit(1);
                         _exit(0);
                 }
 
@@ -452,20 +430,12 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                 int status = 0;
                 waitpid(child, &status, 0);
                 if (WIFEXITED(status) && WEXITSTATUS(status) == 0);
-                else
-                        exit(1);
-                
-                // check destination file (sandbox)
-                char *dest_fname;
-                if (asprintf(&dest_fname, "%s%s", rootdir, fname2) == -1)
-                        errExit("asprintf");
-                EUID_ROOT();
-                if (is_dir(dest_fname)) {
-                        fprintf(stderr, "Error: destination file name is a directory inside the sandbox\n");
+                else {
+                        unlink(tmp_fname);
                         exit(1);
                 }
                 
-                // check write access on destination
+                // copy the temporary file into the destionation file
                 child = fork();
                 if (child < 0)
                         errExit("fork");
@@ -475,25 +445,13 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                                 errExit("chroot");
                         if (chdir("/") < 0)
                                 errExit("chdir");
-
+                        
                         // drop privileges
                         drop_privs(0);
-
-                        if (access(path2, F_OK) == -1) {
-                                FILE *fp = fopen(path2, "w");
-                                if (!fp) {
-                                        fprintf(stderr, "Error: cannot create %s\n", path2);
-                                        exit(1);
-                                }
-                                fclose(fp);
-                        }
-                        else {
-                                if (access(path2, W_OK) == -1) {
-                                        fprintf(stderr, "Error: cannot write %s\n", path2);
-                                        exit(1);
-                                }
-                        }
-
+                        
+                        // copy the file
+                        if (copy_file(tmp_fname, dest_fname, getuid(), getgid(), 0600))
+                                _exit(1);
                         _exit(0);
                 }
 
@@ -501,17 +459,13 @@ void sandboxfs(int op, pid_t pid, const char *path1, const char *path2) {
                 status = 0;
                 waitpid(child, &status, 0);
                 if (WIFEXITED(status) && WEXITSTATUS(status) == 0);
-                else
+                else {
+                        unlink(tmp_fname);
                         exit(1);
-                        
-                // copy file
-                if (arg_debug)
-                        printf("copy %s to %s\n", src_fname, dest_fname);
-                EUID_ROOT();
-                if (copy_file(src_fname, dest_fname, getuid(), getgid(), 0644))
-                        fprintf(stderr, "Error: transfer failed\n");
-                else
-                        printf("Transfer complete\n");
+                }
+                
+                // remove the temporary file
+                unlink(tmp_fname);
                 EUID_USER();
         }
 
diff --git a/src/firejail/netfilter.c b/src/firejail/netfilter.c
index c1f9a2c37..efef45d90 100644
--- a/src/firejail/netfilter.c
+++ b/src/firejail/netfilter.c
@@ -144,6 +144,7 @@ void netfilter(const char *fname) {
 
                 // wipe out environment variables
                 environ = NULL;
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execl(iptables_restore, iptables_restore, NULL);
                 perror("execl");
                 _exit(1);
@@ -163,6 +164,7 @@ void netfilter(const char *fname) {
                         if (setregid(0, 0))
                                 errExit("setregid");
                         environ = NULL;
+                        assert(getenv("LD_PRELOAD") == NULL);   
                         execl(iptables, iptables, "-vL", NULL);
                         perror("execl");
                         _exit(1);
@@ -257,6 +259,7 @@ void netfilter6(const char *fname) {
 
                 // wipe out environment variables
                 environ = NULL;
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execl(ip6tables_restore, ip6tables_restore, NULL);
                 perror("execl");
                 _exit(1);
diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c
index 020e70b80..8aa2fe53f 100644
--- a/src/firejail/run_symlink.c
+++ b/src/firejail/run_symlink.c
@@ -106,6 +106,7 @@ void run_symlink(int argc, char **argv) {
                 a[i + 2] = argv[i + 1];
         }
         a[i + 2] = NULL;
+        assert(getenv("LD_PRELOAD") == NULL);   
         execvp(a[0], a); 
 
         perror("execvp");
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 3942e4da6..e3c95283d 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -260,6 +260,7 @@ void start_audit(void) {
         char *audit_prog;
         if (asprintf(&audit_prog, "%s/firejail/faudit", LIBDIR) == -1)
                 errExit("asprintf");
+        assert(getenv("LD_PRELOAD") == NULL);   
         execl(audit_prog, audit_prog, NULL);
         perror("execl");
         exit(1);
@@ -268,6 +269,15 @@ void start_audit(void) {
 void start_application(void) {
 //if (setsid() == -1)
 //errExit("setsid");
+
+        // set environment
+        env_defaults();
+        env_apply();
+        if (arg_debug) {
+                printf("starting application\n");
+                printf("LD_PRELOAD=%s\n", getenv("LD_PRELOAD"));
+        }
+        
         //****************************************
         // audit
         //****************************************
@@ -787,12 +797,6 @@ assert(0);
                 }
         }
         
-        // set environment
-        env_defaults();
-        
-        // set user-supplied environment variables
-        env_apply();
-
         // set nice
         if (arg_nice) {
                 errno = 0;
diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c
index 6499b7005..a5a067090 100644
--- a/src/firejail/sbox.c
+++ b/src/firejail/sbox.c
@@ -165,6 +165,7 @@ int sbox_run(unsigned filter, int num, ...) {
                 else if (filter & SBOX_USER)
                         drop_privs(1);
 
+                assert(getenv("LD_PRELOAD") == NULL);   
                 if (arg[0])     // get rid of scan-build warning
                         execvp(arg[0], arg);
                 else
diff --git a/src/firejail/util.c b/src/firejail/util.c
index 9752504e5..a7712441e 100644
--- a/src/firejail/util.c
+++ b/src/firejail/util.c
@@ -176,12 +176,6 @@ int copy_file(const char *srcname, const char *destname, uid_t uid, gid_t gid, m
         assert(srcname);
         assert(destname);
 
-        struct stat s;
-        if (stat(destname, &s) == 0) {
-                fprintf(stderr, "Error: file %s already exists\n", destname);
-                return -1;
-        }
-
         // open source
         int src = open(srcname, O_RDONLY);
         if (src < 0) {
diff --git a/src/firejail/x11.c b/src/firejail/x11.c
index c79f1a74e..6cba95501 100644
--- a/src/firejail/x11.c
+++ b/src/firejail/x11.c
@@ -311,7 +311,8 @@ void x11_start_xephyr(int argc, char **argv) {
         if (server == 0) {
                 if (arg_debug)
                         printf("Starting xephyr...\n");
-        
+
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execvp(server_argv[0], server_argv);
                 perror("execvp");
                 _exit(1);
@@ -353,6 +354,7 @@ void x11_start_xephyr(int argc, char **argv) {
                 if (!arg_quiet)
                         printf("\n*** Attaching to Xephyr display %d ***\n\n", display);
 
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execvp(jail_argv[0], jail_argv);
                 perror("execvp");
                 _exit(1);
@@ -432,6 +434,7 @@ void x11_start_xpra(int argc, char **argv) {
                         dup2(fd_null,2);
                 }
         
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execvp(server_argv[0], server_argv);
                 perror("execvp");
                 _exit(1);
@@ -478,6 +481,7 @@ void x11_start_xpra(int argc, char **argv) {
                 if (!arg_quiet)
                         printf("\n*** Attaching to xpra display %d ***\n\n", display);
 
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execvp(attach_argv[0], attach_argv);
                 perror("execvp");
                 _exit(1);
@@ -508,6 +512,7 @@ void x11_start_xpra(int argc, char **argv) {
         if (jail < 0)
                 errExit("fork");
         if (jail == 0) {
+                assert(getenv("LD_PRELOAD") == NULL);   
                 if (firejail_argv[0]) // shut up llvm scan-build
                         execvp(firejail_argv[0], firejail_argv);
                 perror("execvp");
@@ -534,6 +539,7 @@ void x11_start_xpra(int argc, char **argv) {
                                         dup2(fd_null,1);
                                         dup2(fd_null,2);
                                 }
+                                assert(getenv("LD_PRELOAD") == NULL);   
                                 execvp(stop_argv[0], stop_argv);
                                 perror("execvp");
                                 _exit(1);
@@ -664,11 +670,12 @@ void x11_xorg(void) {
                         errExit("setreuid");
                 if (setregid(0, 0) < 0)
                         errExit("setregid");
-                
+
                 char *display = getenv("DISPLAY");
                 if (!display)
                         display = ":0.0";
-                        
+                
+                assert(getenv("LD_PRELOAD") == NULL);   
                 execlp("/usr/bin/xauth", "/usr/bin/xauth", "-f", RUN_XAUTHORITY_SEC_FILE,
                         "generate", display, "MIT-MAGIC-COOKIE-1", "untrusted", NULL);