Commit bab9df35 authored by Gerd Hoffmann's avatar Gerd Hoffmann
Browse files

usb-mtp: use O_NOFOLLOW and O_CLOEXEC. 



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 a successful symlink attack
requires swapping an existing file or directory below rootdir for a
symlink and winning the race against the inotify notification to qemu.

Fixes: CVE-2018-16872
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: Bandan Das <bsd@redhat.com>
Reported-by: default avatarMichael Hanselmann <public@hansmi.ch>
Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
Reviewed-by: default avatarMichael Hanselmann <public@hansmi.ch>
Message-id: 20181213122511.13853-1-kraxel@redhat.com
parent b7d3a7e1

hw/usb/dev-mtp.c

+9 −4
Original line number Diff line number Diff line
@@ -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);