Re: [PATCH v2] 9pfs: prevent opening special files (CVE-2023-2861)

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Tuesday, June 6, 2023 8:48:49 PM CEST Michael Tokarev wrote:
> 06.06.2023 16:57, Christian Schoenebeck wrote:
[...]
> > +    /* CVE-2023-2861: Prohibit opening any special file directly on host
> > +     * (especially device files), as a compromised client could potentially
> > +     * gain access outside exported tree under certain, unsafe setups. We
> > +     * expect client to handle I/O on special files exclusively on guest side.
> > +     */
> > +    if (qemu_fstat(fd, &stbuf) < 0) {
> > +        close_preserve_errno(fd);
> > +        return -1;
> > +    }
> > +    if (!S_ISREG(stbuf.st_mode) && !S_ISDIR(stbuf.st_mode)) {
> > +        /* Tcreate and Tlcreate 9p messages mandate to immediately open the
> > +         * created file for I/O. So this is not (necessarily) due to a broken
> > +         * client, and hence no error message is to be reported in this case.
> > +         */
> > +        if (!(flags & O_CREAT)) {
> > +            error_report_once(
> > +                "9p: broken or compromised client detected; attempt to open "
> > +                "special file (i.e. neither regular file, nor directory)"
> > +            );
> > +        }
> > +        close(fd);
> > +        errno = ENXIO;
> > +        return -1;
> > +    }
> > +
> 
> can't we re-use this same code used in two places, placing it into an inline
> function, such as is_file_regular_or_dir(fd) ?  It smells like a very good
> candidate for implementing it in a single place..

Yeah, my plan was to officially deprecate 9p proxy subsequently, so I didn't
care too much about code duplication, but I guess you are right, it is simple
enough to do it right.

Best regards,
Christian Schoenebeck