Skip to content

Commit

Permalink
fusefronted: close race between mknod and chown
Browse files Browse the repository at this point in the history
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
#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.
  • Loading branch information
rfjakob committed Nov 26, 2017
1 parent 1bb47b6 commit 05d4679
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions internal/fusefrontend/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit 05d4679

Please sign in to comment.