Re: [Qemu-devel] [PATCH v2 00/28] 9pfs: local: fix vulnerability to symlink attacks

[Greg Kurz <groug@kaod.org>]

[-- Attachment #1: Type: text/plain, Size: 4179 bytes --]

On Sun, 26 Feb 2017 23:41:32 +0100
Greg Kurz <groug@kaod.org> wrote:

> This series tries to fix CVE-2016-9602 reported by Jann Horn of Google
> Project Zero:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1413929
> 
> This vulnerability affects all accesses to the underlying filesystem in
> the "local" backend code.
> 
> If QEMU is started with:
> 
> -fsdev local,security_model=<passthrough|none>,path=/foo/bar
> 
> then the guest can cause QEMU to create symlinks in /foo/bar.
> 
> This causes accesses to any path /foo/bar/some/path to be unsafe, since
> untrusted code within the guest (or in another guest sharing the same
> virtfs folder) could change some/path to point to a random path of the
> host filesystem.
> 
> The core problem is that the "local" backend relies on path-based syscalls
> to access the underlying filesystem. All path-based syscalls are vulnerable
> to this issue, even open(O_NOFOLLOW) or syscalls that explicitly don't
> dereference symlinks, since the kernel only checks the rightmost element of
> the path. Depending on the privilege level of the QEMU process, a guest can
> end up opening, renaming, changing ACLs, unlinking... files on the host
> filesystem.
> 
> The right way to address this is to use "at" variants of all syscalls in
> the "local" backend code. This requires to open directories without
> traversing any symlink in the intermediate path elements. There was a
> tentative to introduce an O_BENEATH flag for openat() that would address
> this:
> 
> https://patchwork.kernel.org/patch/7007181/
> 
> Unfortunately this never got merged.  An alternative is to walk through all
> path elements manually with openat(O_NOFOLLOW).
> 
> v2:
> 	- /proc based implementation for xattr code (fixes metadata perf drop
>           observed with v1)
>         - some code refactoring
> 
> Stefan.
> 
> I had to rework some patches you had already reviewed, please consider
> giving your Reviewed-by again if the changes are ok.
> 
> Thanks.
> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (28):
>       9pfs: local: move xattr security ops to 9p-xattr.c
>       9pfs: remove side-effects in local_init()
>       9pfs: remove side-effects in local_open() and local_opendir()
>       9pfs: introduce openat_nofollow() helper
>       9pfs: local: keep a file descriptor on the shared folder
>       9pfs: local: open/opendir: don't follow symlinks
>       9pfs: local: lgetxattr: don't follow symlinks
>       9pfs: local: llistxattr: don't follow symlinks
>       9pfs: local: lsetxattr: don't follow symlinks
>       9pfs: local: lremovexattr: don't follow symlinks
>       9pfs: local: unlinkat: don't follow symlinks
>       9pfs: local: remove: don't follow symlinks
>       9pfs: local: utimensat: don't follow symlinks
>       9pfs: local: statfs: don't follow symlinks
>       9pfs: local: truncate: don't follow symlinks
>       9pfs: local: readlink: don't follow symlinks
>       9pfs: local: lstat: don't follow symlinks
>       9pfs: local: renameat: don't follow symlinks
>       9pfs: local: rename: use renameat
>       9pfs: local: improve error handling in link op
>       9pfs: local: link: don't follow symlinks
>       9pfs: local: chmod: don't follow symlinks
>       9pfs: local: chown: don't follow symlinks
>       9pfs: local: symlink: don't follow symlinks
>       9pfs: local: mknod: don't follow symlinks
>       9pfs: local: mkdir: don't follow symlinks
>       9pfs: local: open2: don't follow symlinks
>       9pfs: local: drop unused code
> 
> 
>  hw/9pfs/9p-local.c      | 1023 ++++++++++++++++++++++++++---------------------
>  hw/9pfs/9p-local.h      |   20 +
>  hw/9pfs/9p-posix-acl.c  |   44 --
>  hw/9pfs/9p-util.c       |   65 +++
>  hw/9pfs/9p-util.h       |   52 ++
>  hw/9pfs/9p-xattr-user.c |   24 -
>  hw/9pfs/9p-xattr.c      |  166 +++++++-
>  hw/9pfs/9p-xattr.h      |   87 +---
>  hw/9pfs/Makefile.objs   |    2 
>  9 files changed, 887 insertions(+), 596 deletions(-)
>  create mode 100644 hw/9pfs/9p-local.h
>  create mode 100644 hw/9pfs/9p-util.c
>  create mode 100644 hw/9pfs/9p-util.h
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]