From 320ebe08f9039a81b4dcf53bf64ba6fddd383710 Mon Sep 17 00:00:00 2001
From: smitsohu <smitsohu@gmail.com>
Date: Wed, 18 Sep 2019 17:15:05 +0200
Subject: break out fs_tracefile function

---
 src/firejail/firejail.h     |  1 +
 src/firejail/fs_trace.c     | 76 +++++++++++++++++++++++----------------------
 src/firejail/fs_whitelist.c |  5 +--
 src/firejail/sandbox.c      | 10 ++++--
 4 files changed, 51 insertions(+), 41 deletions(-)

(limited to 'src')

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
-- 
cgit v1.2.3-54-g00ecf