Re: [PATCH] virtiofsd: prevent opening of special files (CVE-2020-35517)

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]

On Fri, Jan 22, 2021 at 10:40:54AM -0500, Vivek Goyal wrote:
> On Thu, Jan 21, 2021 at 02:44:29PM +0000, Stefan Hajnoczi wrote:
> > A well-behaved FUSE client does not attempt to open special files with
> > FUSE_OPEN because they are handled on the client side (e.g. device nodes
> > are handled by client-side device drivers).
> > 
> > The check to prevent virtiofsd from opening special files is missing in
> > a few cases, most notably FUSE_OPEN. A malicious client can cause
> > virtiofsd to open a device node, potentially allowing the guest to
> > escape. This can be exploited by a modified guest device driver. It is
> > not exploitable from guest userspace since the guest kernel will handle
> > special files inside the guest instead of sending FUSE requests.
> > 
> > This patch adds the missing checks to virtiofsd. This is a short-term
> > solution because it does not prevent a compromised virtiofsd process
> > from opening device nodes on the host.
> > 
> > Reported-by: Alex Xu <alex@alxu.ca>
> > Fixes: CVE-2020-35517
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> It looks good to me. I see there is another openat() instance in
> lo_opendir(). May be we can convert it to use lo_inode_open() as well?

I thought so too, but this one is interesting:

  fd = openat(lo_fd(req, ino), ".", O_RDONLY);

Using "." here is basically equivalent to O_DIRECTORY!

Therefore this always fails on special files. That's why I ended up
leaving it unchanged.

  $ cat a.c
  #define _GNU_SOURCE
  #include <sys/types.h>
  #include <sys/stat.h>
  #include <fcntl.h>
  #include <unistd.h>
  #include <stdio.h>
  #include <stdlib.h>

  int main(int argc, char **argv)
  {
      int fd;
      int pfd = open(argv[1], O_PATH);

      if (pfd < 0) {
          perror("open");
          return EXIT_FAILURE;
      }

      fd = openat(pfd, ".", O_RDONLY);
      if (fd < 0) {
          perror("openat");
          return EXIT_FAILURE;
      }

      close(fd);
      close(pfd);
      return EXIT_SUCCESS;
  }
  $ gcc -o a a.c
  $ ./a /tmp
  <--- no error
  $ ./a /tmp/a.c
  openat: Not a directory
  $ ./a /dev/stdin
  openat: Not a directory

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]