break out fs_tracefile function

4 files changed, 51 insertions, 41 deletions

diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 4a59522bf..a6377261f 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -560,6 +560,7 @@ void caps_drop_dac_override(void);
 
 // fs_trace.c
 void fs_trace_preload(void);
+void fs_tracefile(void);
 void fs_trace(void);
 
 // fs_hostname.c
diff --git a/src/firejail/fs_trace.c b/src/firejail/fs_trace.c
index 9ade0bdc3..c1b821cce 100644
--- a/src/firejail/fs_trace.c
+++ b/src/firejail/fs_trace.c
@@ -41,44 +41,46 @@ void fs_trace_preload(void) {
                 fclose(fp);
                 fs_logger("touch /etc/ld.so.preload");
         }
-        if (arg_tracefile) {
-                if (arg_debug)
-                        printf("Creating an empty trace log file: %s\n", arg_tracefile);
-                // create a bind mounted trace logfile that the sandbox can see
-                EUID_USER();
-                int fd = open(arg_tracefile, O_CREAT|O_RDWR, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
-                if (fd == -1) {
-                        perror("open");
-                        fprintf(stderr, "Error: cannot open trace log file %s\n", arg_tracefile);
-                        exit(1);
-                }
-                if (fstat(fd, &s) == -1)
-                        errExit("fstat");
-                if (!S_ISREG(s.st_mode)) {
-                        fprintf(stderr, "Error: cannot write trace log: %s is no regular file\n", arg_tracefile);
-                        exit(1);
-                }
-                if (ftruncate(fd, 0) == -1)
-                        errExit("ftruncate");
-                EUID_ROOT();
-                FILE *fp = fopen(RUN_TRACE_FILE, "w");
-                if (!fp)
-                        errExit("fopen " RUN_TRACE_FILE);
-                fclose(fp);
-                fs_logger2("touch ", arg_tracefile);
-                // mount using the symbolic link in /proc/self/fd
-                if (arg_debug)
-                        printf("Bind mount %s to %s\n", arg_tracefile, RUN_TRACE_FILE);
-                char *proc;
-                if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
-                        errExit("asprintf");
-                if (mount(proc, RUN_TRACE_FILE, NULL, MS_BIND|MS_REC, NULL) < 0)
-                        errExit("mount bind " RUN_TRACE_FILE);
-                free(proc);
-                close(fd);
-                // now that RUN_TRACE_FILE is user-writable, mount it noexec
-                fs_remount(RUN_TRACE_FILE, MOUNT_NOEXEC, 0);
+}
+
+void fs_tracefile(void) {
+        // create a bind mounted trace logfile that the sandbox can see
+        if (arg_debug)
+                printf("Creating an empty trace log file: %s\n", arg_tracefile);
+        EUID_USER();
+        int fd = open(arg_tracefile, O_CREAT|O_WRONLY|O_CLOEXEC, S_IRUSR | S_IWRITE | S_IRGRP | S_IROTH);
+        if (fd == -1) {
+                perror("open");
+                fprintf(stderr, "Error: cannot open trace log file %s for writing\n", arg_tracefile);
+                exit(1);
+        }
+        struct stat s;
+        if (fstat(fd, &s) == -1)
+                errExit("fstat");
+        if (!S_ISREG(s.st_mode)) {
+                fprintf(stderr, "Error: cannot write trace log: %s is no regular file\n", arg_tracefile);
+                exit(1);
         }
+        if (ftruncate(fd, 0) == -1)
+                errExit("ftruncate");
+        EUID_ROOT();
+        FILE *fp = fopen(RUN_TRACE_FILE, "w");
+        if (!fp)
+                errExit("fopen " RUN_TRACE_FILE);
+        fclose(fp);
+        fs_logger2("touch ", arg_tracefile);
+        // mount using the symbolic link in /proc/self/fd
+        if (arg_debug)
+                printf("Bind mount %s to %s\n", arg_tracefile, RUN_TRACE_FILE);
+        char *proc;
+        if (asprintf(&proc, "/proc/self/fd/%d", fd) == -1)
+                errExit("asprintf");
+        if (mount(proc, RUN_TRACE_FILE, NULL, MS_BIND|MS_REC, NULL) < 0)
+                errExit("mount bind " RUN_TRACE_FILE);
+        free(proc);
+        close(fd);
+        // now that RUN_TRACE_FILE is user-writable, mount it noexec
+        fs_remount(RUN_TRACE_FILE, MOUNT_NOEXEC, 0);
 }
 
 void fs_trace(void) {
diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c
index fa93751cc..d2ea495ed 100644
--- a/src/firejail/fs_whitelist.c
+++ b/src/firejail/fs_whitelist.c
@@ -290,7 +290,8 @@ static void whitelist_path(ProfileEntry *entry) {
 
         fs_logger2("whitelist", path);
 
-        // mount via the link in /proc/self/fd
+        // in order to make this mount resilient against symlink attacks, use
+        // a magic link in /proc/self/fd instead of mounting on path directly
         char *proc;
         if (asprintf(&proc, "/proc/self/fd/%d", fd3) == -1)
                 errExit("asprintf");
@@ -315,7 +316,7 @@ static void whitelist_path(ProfileEntry *entry) {
         //  - there should be more than one '/' char in dest string
         if (mptr->dir == strrchr(mptr->dir, '/'))
                 errLogExit("invalid whitelist mount");
-        // confirm the right file was mounted
+        // confirm the right file was mounted by comparing device and inode numbers
         int fd4 = safe_fd(path, O_PATH|O_NOFOLLOW|O_CLOEXEC);
         if (fd4 == -1)
                 errExit("safe_fd");
diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c
index 288726d22..51c531159 100644
--- a/src/firejail/sandbox.c
+++ b/src/firejail/sandbox.c
@@ -800,8 +800,11 @@ int sandbox(void* sandbox_arg) {
         }
 
         // trace pre-install
-        if (need_preload)
+        if (need_preload) {
                 fs_trace_preload();
+                if (arg_tracefile && !cfg.chrootdir)
+                        fs_tracefile();
+        }
 
         // store hosts file
         if (cfg.hosts_file)
@@ -817,8 +820,11 @@ int sandbox(void* sandbox_arg) {
                 //****************************
                 // trace pre-install, this time inside chroot
                 //****************************
-                if (need_preload)
+                if (need_preload) {
                         fs_trace_preload();
+                        if (arg_tracefile)
+                                fs_tracefile();
+                }
         }
         else
 #endif