From 05d46798f2f54f075accc3d9253f8f538305cebe Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 26 Nov 2017 21:59:24 +0100 Subject: [PATCH] fusefronted: close race between mknod and chown If the user manages to replace the directory with a symlink at just the right time, we could be tricked into chown'ing the wrong file. This change fixes the race by using fchownat. Scenario, as described by @slackner at https://github.com/rfjakob/gocryptfs/issues/177 : 1. Create a forward mount point with `plaintextnames` enabled 2. Mount as root user with `allow_other` 3. For testing purposes create a file `/tmp/file_owned_by_root` which is owned by the root user 4. As a regular user run inside of the GoCryptFS mount: ``` mkdir tempdir mknod tempdir/file_owned_by_root p & mv tempdir tempdir2 ln -s /tmp tempdir ``` When the steps are done fast enough and in the right order (run in a loop!), the device file will be created in `tempdir`, but the `lchown` will be executed by following the symlink. As a result, the ownership of the file located at `/tmp/file_owned_by_root` will be changed. --- internal/fusefrontend/fs.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go index b4819fd1..04687bb0 100644 --- a/internal/fusefrontend/fs.go +++ b/internal/fusefrontend/fs.go @@ -10,6 +10,8 @@ import ( "syscall" "time" + "golang.org/x/sys/unix" + "github.com/hanwen/go-fuse/fuse" "github.com/hanwen/go-fuse/fuse/nodefs" "github.com/hanwen/go-fuse/fuse/pathfs" @@ -280,15 +282,14 @@ func (fs *FS) Mknod(path string, mode uint32, dev uint32, context *fuse.Context) if err != nil { return fuse.ToStatus(err) } + dirfd, err := os.Open(filepath.Dir(cPath)) + if err != nil { + return fuse.ToStatus(err) + } + defer dirfd.Close() // Create ".name" file to store long file name cName := filepath.Base(cPath) if nametransform.IsLongContent(cName) { - var dirfd *os.File - dirfd, err = os.Open(filepath.Dir(cPath)) - if err != nil { - return fuse.ToStatus(err) - } - defer dirfd.Close() err = fs.nameTransform.WriteLongName(dirfd, cName, path) if err != nil { return fuse.ToStatus(err) @@ -300,16 +301,17 @@ func (fs *FS) Mknod(path string, mode uint32, dev uint32, context *fuse.Context) } } else { // Create regular device node - err = syscall.Mknod(cPath, mode, int(dev)) + err = syscallcompat.Mknodat(int(dirfd.Fd()), cName, mode, int(dev)) } if err != nil { return fuse.ToStatus(err) } // Set owner if fs.args.PreserveOwner { - err = os.Lchown(cPath, int(context.Owner.Uid), int(context.Owner.Gid)) + syscall.Fchownat(int(dirfd.Fd()), cPath, int(context.Owner.Uid), + int(context.Owner.Gid), unix.AT_SYMLINK_NOFOLLOW) if err != nil { - tlog.Warn.Printf("Mknod: Lchown failed: %v", err) + tlog.Warn.Printf("Mknod: Fchownat failed: %v", err) } } return fuse.OK