On Wed, 27 Jan 2021 10:25:28 +0100
Miklos Szeredi <mszer...@redhat.com> wrote:
> On Tue, Jan 26, 2021 at 6:18 PM Greg Kurz <gr...@kaod.org> wrote:
> >
> > On Tue, 26 Jan 2021 10:35:02 +0000
> > Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> > The patch looks pretty good to me. It just seems to be missing a change in
> > lo_create():
> >
> > fd = openat(parent_inode->fd, name, (fi->flags | O_CREAT) & ~O_NOFOLLOW,
> > mode);
> >
> > A malicious guest could have created anything called ${name} in this
> > directory
> > before calling FUSE_CREATE and we'll open it blindly, or I'm missing
> > something ?
>
> Right, this seems like an omission.
>
> Also the "& ~O_NOFOLLOW" looks like a copy-paste bug, since unlike
> lo_open(), lo_create() is not opening a proc symlink.
>
> So that should be replaced with "| O_NOFOLLOW"
>
Yes, I've realized that later on. We should definitely enforce O_NOFOLLOW
to avoid symlink escapes.
Then comes the case of special files... A well-known case is the FIFO that
causes openat() to block as described in my response. FWIW, we addressed
this one in 9P by adding O_NONBLOCK and fixing the flags to the client
expectation with fcntl(F_SETFL). But this is just a protection against
being blocked. Blindly opening a special file can lead to any kind of
troubles you can think of... so it really looks that the only sane way
to be safe from such an attack is to forbid openat() of special files at
the filesystem level.
> Thanks,
> Miklos
>
