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
