Description: fix privilege escalation via mountpoint race conditions
Author: Dan Rosenberg
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/ecryptfs-utils/+bug/732628

Index: src/utils/mount.ecryptfs_private.c
===================================================================
--- src/utils/mount.ecryptfs_private.c.orig
+++ src/utils/mount.ecryptfs_private.c
@@ -24,6 +24,7 @@
  * 
  */
 
+#define _GNU_SOURCE
 
 #include <sys/mount.h>
 #include <sys/stat.h>
@@ -36,6 +37,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <syslog.h>
+#include <fcntl.h>
 
 /* Perhaps a future version of this program will allow these to be configurable
  * by the system administrator (or user?) at run time.  For now, these are set
@@ -134,6 +136,46 @@ char *fetch_sig(char *pw_dir) {
 	return sig;
 }
 
+int check_ownership_mnt(int uid, char **mnt) {
+/* Check ownership of mount point, chdir into it, and
+ * canonicalize the path for use in mtab updating.
+ * Return 0 if everything is in order, 1 on error.
+ */
+	struct stat s;
+	char *cwd;
+
+	/* From here on, we'll refer to "." as our mountpoint, to avoid
+	* races.
+	*/
+	if (chdir(*mnt) != 0) {
+		fputs("Cannot chdir into mountpoint.\n", stderr);
+		return 1;
+	}
+	if (stat(".", &s) != 0) {
+		fputs("Cannot examine mountpoint.\n", stderr);
+		return 1;
+	}
+	if (!S_ISDIR(s.st_mode)) {
+		fputs("Mountpoint is not a directory.\n", stderr);
+		return 1;
+	}
+	if (s.st_uid != uid) {
+		fputs("You do not own that mountpoint.\n", stderr);
+		return 1;
+	}
+
+	/* Canonicalize our pathname based on the current directory to
+	* avoid races.
+	*/
+	cwd = getcwd(NULL, 0);
+	if (!cwd) {
+		fputs("Failed to get current directory\n", stderr);
+		return 1;
+	}
+	*mnt = cwd;
+	return 0;
+}
+
 
 int check_ownerships(int uid, char *dev, char *mnt) {
 /* Check ownership of device and mount point.
@@ -229,30 +271,77 @@ int update_mtab(char *dev, char *mnt, ch
 /* Update /etc/mtab with new mount entry.
  * Return 0 on success, 1 on failure.
  */
-	FILE *fh;
-	struct mntent m;
-	fh = setmntent("/etc/mtab", "a");
-	if (fh == NULL) {
+	int fd;
+	FILE *old_mtab, *new_mtab;
+	struct mntent *old_ent, new_ent;
+
+	/* Make an attempt to play nice with other mount helpers
+	* by creating an /etc/mtab~ lock file. Of course this
+	* only works if those other helpers actually check for
+	* this.
+	*/
+	fd = open("/etc/mtab~", O_RDONLY | O_CREAT | O_EXCL, 0644);
+	if (fd < 0) {
+		perror("open");
+		return 1;
+	}
+	close(fd);
+
+	old_mtab = setmntent("/etc/mtab", "r");
+	if (old_mtab == NULL) {
 		perror("setmntent");
-		/* Unmount if mtab cannot be updated */
-		umount(mnt);
 		return 1;
 	}
-	m.mnt_fsname = dev;
-	m.mnt_dir = mnt;
-	m.mnt_type = FSTYPE;
-	m.mnt_opts = opt;
-	m.mnt_freq = 0;
-	m.mnt_passno = 0;
-	if (addmntent(fh, &m) != 0) {
+
+	new_mtab = setmntent("/etc/mtab.tmp", "w");
+	if (new_mtab == NULL) {
+		perror("setmntent");
+		goto fail_early;
+	}
+
+	while (old_ent = getmntent(old_mtab)) {
+		if (addmntent(new_mtab, old_ent) != 0) {
+			perror("addmntent");
+			goto fail;
+		}
+	}
+	endmntent(old_mtab);
+
+	new_ent.mnt_fsname = dev;
+	new_ent.mnt_dir = mnt;
+	new_ent.mnt_type = FSTYPE;
+	new_ent.mnt_opts = opt;
+	new_ent.mnt_freq = 0;
+	new_ent.mnt_passno = 0;
+
+	if (addmntent(new_mtab, &new_ent) != 0) {
 		perror("addmntent");
-		endmntent(fh);
-		/* Unmount if mtab cannot be updated */
-		umount(mnt);
-		return 1;
+		goto fail;
 	}
-	endmntent(fh);
+
+	if (fchmod(fileno(new_mtab), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) < 0) {
+		perror("fchmod");
+		goto fail;
+	}
+	endmntent(new_mtab);
+
+	if (rename("/etc/mtab.tmp", "/etc/mtab") < 0) {
+		perror("rename");
+		goto fail_late;
+	}
+
+	unlink("/etc/mtab~");
+
 	return 0;
+
+fail:
+	endmntent(new_mtab);
+fail_late:
+	unlink("/etc/mtab.tmp");
+fail_early:
+	endmntent(old_mtab);
+	unlink("/etc/mtab~");
+	return 1;
 }
 
 
@@ -351,8 +440,9 @@ int main(int argc, char *argv[]) {
 		return 1;
 	}
 
-	/* Check ownership of dev and mnt */
-	if (check_ownerships(uid, dev, mnt) != 0) {
+	/* Check ownership of the mountpoint. From here on, dest refers
+	 * to a canonicalized path, and the mountpoint is the cwd. */
+	if (check_ownership_mnt(uid, &mnt) != 0) {
 		return 1;
 	}
 
@@ -372,7 +462,7 @@ int main(int argc, char *argv[]) {
 		 */
 		setreuid(-1, 0);
 		/* Perform mount */
-		if (mount(dev, mnt, FSTYPE, 0, opt) == 0) {
+		if (mount(dev, ".", FSTYPE, 0, opt) == 0) {
 			if (update_mtab(dev, mnt, opt) != 0) {
 				return 1;
 			}
@@ -391,7 +481,12 @@ int main(int argc, char *argv[]) {
 		 * Do not use the umount.ecryptfs helper (-i).
  		 */
 		setresuid(0,0,0);
-		execl("/bin/umount", "umount", "-i", mnt, NULL);
+
+		/* Since we're doing a lazy unmount anyway, just unmount the current
+		 * directory. This avoids a lot of complexity in dealing with race
+		 * conditions, and guarantees that we're only unmounting a filesystem
+		 * that we own. */
+		execl("/bin/umount", "umount", "-i", "-l", ".", NULL);
 		perror("execl unmount failed");
 		return 1;
 	}
