[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] usb-mtp: use O_NOFOLLOW and O_CLOEXEC.
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH] usb-mtp: use O_NOFOLLOW and O_CLOEXEC. |
Date: |
Thu, 13 Dec 2018 12:37:03 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
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 <address@hidden>
> Cc: Bandan Das <address@hidden>
> Reported-by: Michael Hanselmann <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
> 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 :|