From b22cc03c7516b2003880db8375d26c76d6dff093 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher
Date: Tue, 8 Jan 2019 21:50:10 +0100
Subject: fusefrontend: -allow_other: set file mode *after* chown in Create()

Reported by @slackner at https://github.com/rfjakob/gocryptfs/issues/327 :

Possible race-conditions between file creation and Fchownat

* Assume a system contains a gocryptfs mount as root user
  with -allow_other
* As a regular user create a new file with mode containing
  the SUID flag and write access for other users
* Before gocryptfs executes the Fchownat call, try to open
  the file again, write some exploit code to it, and try to run it.

For a short time, the file is owned by root and has the SUID flag, so
this is pretty dangerous.
---
 internal/fusefrontend/fs.go | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

(limited to 'internal')

diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go
index 7671215..7beeb4f 100644
--- a/internal/fusefrontend/fs.go
+++ b/internal/fusefrontend/fs.go
@@ -239,6 +239,14 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte
 		return nil, fuse.ToStatus(err)
 	}
 	defer syscall.Close(dirfd)
+	// Don't set full mode before we have set the correct owner. Files with SUID/SGID
+	// mode belonging to the wrong owner would be a security risk. Even for other
+	// modes, we don't want anyone else to open the file in the meantime: the fd would
+	// stay open and could later be used to read the file.
+	origMode := mode
+	if fs.args.PreserveOwner {
+		mode = 0000
+	}
 	fd := -1
 	// Handle long file name
 	if !fs.args.PlaintextNames && nametransform.IsLongContent(cName) {
@@ -253,7 +261,6 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte
 			nametransform.DeleteLongNameAt(dirfd, cName)
 			return nil, fuse.ToStatus(err)
 		}
-
 	} else {
 		// Create content, normal (short) file name
 		fd, err = syscallcompat.Openat(dirfd, cName, newFlags|syscall.O_CREAT|syscall.O_EXCL, mode)
@@ -271,7 +278,18 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte
 	if fs.args.PreserveOwner {
 		err = syscall.Fchown(fd, int(context.Owner.Uid), int(context.Owner.Gid))
 		if err != nil {
-			tlog.Warn.Printf("Create: Fchown() failed: %v", err)
+			tlog.Warn.Printf("Create %q: Fchown %d:%d failed: %v", cName, context.Owner.Uid, context.Owner.Gid, err)
+			// In case of a failure, we don't want to proceed setting more
+			// permissive modes.
+			syscall.Close(fd)
+			return nil, fuse.ToStatus(err)
+		}
+	}
+	// Set mode
+	if mode != origMode {
+		err = syscall.Fchmod(fd, origMode)
+		if err != nil {
+			tlog.Warn.Printf("Create %q: Fchmod %#o -> %#o failed: %v", cName, mode, origMode, err)
 		}
 	}
 	f := os.NewFile(uintptr(fd), cName)
-- 
cgit v1.2.3