On Thu, Dec 13, 2018 at 01:25:11PM +0100, Gerd Hoffmann wrote: > Open files and directories with O_NOFOLLOW to avoid symlinks attacks. > While being at it also add O_CLOEXEC. > > usb-mtp only handles regular files and directories and ignores > everything else, so users should not see a difference. > > Because qemu ignores symlinks carrying out an successfull symlink attack > requires swapping an existing file or directory below rootdir for a > symlink and winning the race against the inotify notification to qemu. > > Note that the impact of this bug is rather low when qemu is managed by > libvirt due to qemu running sandboxed, so there isn't much you can gain > access to that way.
It is almost non-existant because libvirt doesn't support the MTP device at all yet, so no guest will have it unless the user tried CLI arg passthrough in libvirt :-) > > Fixes: CVE-2018-pjp-please-get-one > Cc: Prasad J Pandit <ppan...@redhat.com> > Cc: Bandan Das <b...@redhat.com> > Reported-by: Michael Hanselmann <pub...@hansmi.ch> > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > hw/usb/dev-mtp.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c > index 100b7171f4..36c43b8c20 100644 > --- a/hw/usb/dev-mtp.c > +++ b/hw/usb/dev-mtp.c > @@ -653,13 +653,18 @@ static void usb_mtp_object_readdir(MTPState *s, > MTPObject *o) > { > struct dirent *entry; > DIR *dir; > + int fd; > > if (o->have_children) { > return; > } > o->have_children = true; > > - dir = opendir(o->path); > + fd = open(o->path, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); > + if (fd < 0) { > + return; > + } > + dir = fdopendir(fd); > if (!dir) { > return; > } > @@ -1007,7 +1012,7 @@ static MTPData *usb_mtp_get_object(MTPState *s, > MTPControl *c, > > trace_usb_mtp_op_get_object(s->dev.addr, o->handle, o->path); > > - d->fd = open(o->path, O_RDONLY); > + d->fd = open(o->path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); > if (d->fd == -1) { > usb_mtp_data_free(d); > return NULL; > @@ -1031,7 +1036,7 @@ static MTPData *usb_mtp_get_partial_object(MTPState *s, > MTPControl *c, > c->argv[1], c->argv[2]); > > d = usb_mtp_data_alloc(c); > - d->fd = open(o->path, O_RDONLY); > + d->fd = open(o->path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); > if (d->fd == -1) { > usb_mtp_data_free(d); > return NULL; > @@ -1658,7 +1663,7 @@ static void usb_mtp_write_data(MTPState *s) > 0, 0, 0, 0); > goto done; > } > - d->fd = open(path, O_CREAT | O_WRONLY, mask); > + d->fd = open(path, O_CREAT | O_WRONLY | O_CLOEXEC | O_NOFOLLOW, > mask); > if (d->fd == -1) { > usb_mtp_queue_result(s, RES_STORE_FULL, d->trans, > 0, 0, 0, 0); > -- > 2.9.3 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|