From b0fe4d5868382f38b3ffaca0d78b120dcb85eba4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Sun, 11 Jan 2015 05:39:25 +0100
Subject: [PATCH] filecopy: create new file unaccessible to the user until
 fully written

Otherwise source domain can modify (append) the file while the user
already is accessing it. While incoming files should be treated as
untrusted, this problem could allow file modification after the user
makes some sanity checks.
---
 qrexec-lib/libqubes-rpc-filecopy.h |  2 ++
 qrexec-lib/unpack.c                | 33 +++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/qrexec-lib/libqubes-rpc-filecopy.h b/qrexec-lib/libqubes-rpc-filecopy.h
index d566345..6cc9d79 100644
--- a/qrexec-lib/libqubes-rpc-filecopy.h
+++ b/qrexec-lib/libqubes-rpc-filecopy.h
@@ -66,6 +66,8 @@ int copy_file(int outfd, int infd, long long size, unsigned long *crc32);
 const char *copy_file_status_to_str(int status);
 void set_size_limit(unsigned long long new_bytes_limit, unsigned long long new_files_limit);
 void set_verbose(int value);
+/* register open fd to /proc/PID/fd of this process */
+void set_procfs_fd(int value);
 int write_all(int fd, const void *buf, int size);
 int read_all(int fd, void *buf, int size);
 int copy_fd_all(int fdout, int fdin);
diff --git a/qrexec-lib/unpack.c b/qrexec-lib/unpack.c
index d7787d1..9a44e72 100644
--- a/qrexec-lib/unpack.c
+++ b/qrexec-lib/unpack.c
@@ -18,9 +18,19 @@ unsigned long long files_limit = 0;
 unsigned long long total_bytes = 0;
 unsigned long long total_files = 0;
 int verbose = 0;
+int use_tmpfile = 0;
+int procdir_fd = -1;
 
 void send_status_and_crc(int code, const char *last_filename);
 
+/* copy from asm-generic/fcntl.h */
+#ifndef __O_TMPFILE
+#define __O_TMPFILE 020000000
+#endif
+/* a horrid kludge trying to make sure that this will fail on old kernels */
+#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
+#define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT)
+
 void do_exit(int code, const char *last_filename)
 {
 	close(0);
@@ -39,6 +49,12 @@ void set_verbose(int value)
 	verbose = value;
 }
 
+void set_procfs_fd(int value)
+{
+	procdir_fd = value;
+	use_tmpfile = 1;
+}
+
 unsigned long crc32_sum = 0;
 int read_all_with_crc(int fd, void *buf, int size) {
 	int ret;
@@ -88,7 +104,15 @@ void process_one_file_reg(struct file_header *untrusted_hdr,
 			  const char *untrusted_name)
 {
 	int ret;
-	int fdout = open(untrusted_name, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, 0700);	/* safe because of chroot */
+	int fdout;
+
+	/* make the file inaccessible until fully written */
+	fdout = open(".", O_WRONLY | O_TMPFILE, 0700);
+	if (fdout < 0 && errno==ENOENT) {
+		/* if it fails, do not attempt further use - most likely kernel too old */
+		use_tmpfile = 0;
+		fdout = open(untrusted_name, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, 0000);	/* safe because of chroot */
+	}
 	if (fdout < 0)
 		do_exit(errno, untrusted_name);
 	/* sizes are signed elsewhere */
@@ -105,6 +129,13 @@ void process_one_file_reg(struct file_header *untrusted_hdr,
 		else
 			do_exit(errno, untrusted_name);
 	}
+	fdatasync(fdout);
+	if (use_tmpfile) {
+		char fd_str[7];
+		snprintf(fd_str, sizeof(fd_str), "%d", fdout);
+		if (linkat(procdir_fd, fd_str, AT_FDCWD, untrusted_name, AT_SYMLINK_FOLLOW) < 0)
+			do_exit(errno, untrusted_name);
+	}
 	close(fdout);
 	fix_times_and_perms(untrusted_hdr, untrusted_name);
 }