qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 56fc49: 9pfs: local: move xattr security ops


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 56fc49: 9pfs: local: move xattr security ops to 9p-xattr.c
Date: Wed, 01 Mar 2017 06:45:09 -0800

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 56fc494bdcba35d74da27e1d34dbb6db6fa7bd67
      
https://github.com/qemu/qemu/commit/56fc494bdcba35d74da27e1d34dbb6db6fa7bd67
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-xattr.c
    M hw/9pfs/9p-xattr.h

  Log Message:
  -----------
  9pfs: local: move xattr security ops to 9p-xattr.c

These functions are always called indirectly. It really doesn't make sense
for them to sit in a header file.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 00c90bd1c2ff6aabb9ca948a254ba044a403e399
      
https://github.com/qemu/qemu/commit/00c90bd1c2ff6aabb9ca948a254ba044a403e399
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: remove side-effects in local_init()

If this function fails, it should not modify *ctx.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 21328e1e57f526e3f0c2fcd00f10c8aa6e7bc07f
      
https://github.com/qemu/qemu/commit/21328e1e57f526e3f0c2fcd00f10c8aa6e7bc07f
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: remove side-effects in local_open() and local_opendir()

If these functions fail, they should not change *fs. Let's use local
variables to fix this.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 6482a961636d66cc10928dde5d4d908206e5f65a
      
https://github.com/qemu/qemu/commit/6482a961636d66cc10928dde5d4d908206e5f65a
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    A hw/9pfs/9p-util.c
    A hw/9pfs/9p-util.h
    M hw/9pfs/Makefile.objs

  Log Message:
  -----------
  9pfs: introduce relative_openat_nofollow() helper

When using the passthrough security mode, symbolic links created by the
guest are actual symbolic links on the host file system.

Since the resolution of symbolic links during path walk is supposed to
occur on the client side. The server should hence never receive any path
pointing to an actual symbolic link. This isn't guaranteed by the protocol
though, and malicious code in the guest can trick the server to issue
various syscalls on paths whose one or more elements are symbolic links.
In the case of the "local" backend using the "passthrough" or "none"
security modes, the guest can directly create symbolic links to arbitrary
locations on the host (as per spec). The "mapped-xattr" and "mapped-file"
security modes are also affected to a lesser extent as they require some
help from an external entity to create actual symbolic links on the host,
i.e. another guest using "passthrough" mode for example.

The current code hence relies on O_NOFOLLOW and "l*()" variants of system
calls. Unfortunately, this only applies to the rightmost path component.
A guest could maliciously replace any component in a trusted path with a
symbolic link. This could allow any guest to escape a virtfs shared folder.

This patch introduces a variant of the openat() syscall that successively
opens each path element with O_NOFOLLOW. When passing a file descriptor
pointing to a trusted directory, one is guaranteed to be returned a
file descriptor pointing to a path which is beneath the trusted directory.
This will be used by subsequent patches to implement symlink-safe path walk
for any access to the backend.

Symbolic links aren't the only threats actually: a malicious guest could
change a path element to point to other types of file with undesirable
effects:
- a named pipe or any other thing that would cause openat() to block
- a terminal device which would become QEMU's controlling terminal

These issues can be addressed with O_NONBLOCK and O_NOCTTY.

Two helpers are introduced: one to open intermediate path elements and one
to open the rightmost path element.

Suggested-by: Jann Horn <address@hidden>
Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
(renamed openat_nofollow() to relative_openat_nofollow(),
 assert path is relative and doesn't contain '//',
 fixed side-effect in assert, Greg Kurz)
Signed-off-by: Greg Kurz <address@hidden>


  Commit: 0e35a3782948c6154d7fafe9a02a86bc130199c7
      
https://github.com/qemu/qemu/commit/0e35a3782948c6154d7fafe9a02a86bc130199c7
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: keep a file descriptor on the shared folder

This patch opens the shared folder and caches the file descriptor, so that
it can be used to do symlink-safe path walk.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 996a0d76d7e756e4023ef79bc37bfe629b9eaca7
      
https://github.com/qemu/qemu/commit/996a0d76d7e756e4023ef79bc37bfe629b9eaca7
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c
    A hw/9pfs/9p-local.h

  Log Message:
  -----------
  9pfs: local: open/opendir: don't follow symlinks

The local_open() and local_opendir() callbacks are vulnerable to symlink
attacks because they call:

(1) open(O_NOFOLLOW) which follows symbolic links in all path elements but
    the rightmost one
(2) opendir() which follows symbolic links in all path elements

This patch converts both callbacks to use new helpers based on
openat_nofollow() to only open files and directories if they are
below the virtfs shared folder

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 56ad3e54dad6cdcee8668d170df161d89581846f
      
https://github.com/qemu/qemu/commit/56ad3e54dad6cdcee8668d170df161d89581846f
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-posix-acl.c
    M hw/9pfs/9p-util.c
    M hw/9pfs/9p-util.h
    M hw/9pfs/9p-xattr-user.c
    M hw/9pfs/9p-xattr.c
    M hw/9pfs/9p-xattr.h

  Log Message:
  -----------
  9pfs: local: lgetxattr: don't follow symlinks

The local_lgetxattr() callback is vulnerable to symlink attacks because
it calls lgetxattr() which follows symbolic links in all path elements but
the rightmost one.

This patch introduces a helper to emulate the non-existing fgetxattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to lgetxattr().

local_lgetxattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 5507904e362df252f6065cb27d1ff98372db6abc
      
https://github.com/qemu/qemu/commit/5507904e362df252f6065cb27d1ff98372db6abc
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-xattr.c

  Log Message:
  -----------
  9pfs: local: llistxattr: don't follow symlinks

The local_llistxattr() callback is vulnerable to symlink attacks because
it calls llistxattr() which follows symbolic links in all path elements but
the rightmost one.

This patch introduces a helper to emulate the non-existing flistxattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to llistxattr().

local_llistxattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 3e36aba757f76673007a80b3cd56a4062c2e3462
      
https://github.com/qemu/qemu/commit/3e36aba757f76673007a80b3cd56a4062c2e3462
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-posix-acl.c
    M hw/9pfs/9p-util.h
    M hw/9pfs/9p-xattr-user.c
    M hw/9pfs/9p-xattr.c
    M hw/9pfs/9p-xattr.h

  Log Message:
  -----------
  9pfs: local: lsetxattr: don't follow symlinks

The local_lsetxattr() callback is vulnerable to symlink attacks because
it calls lsetxattr() which follows symbolic links in all path elements but
the rightmost one.

This patch introduces a helper to emulate the non-existing fsetxattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to lsetxattr().

local_lsetxattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 72f0d0bf51362011c4d841a89fb8f5cfb16e0bf3
      
https://github.com/qemu/qemu/commit/72f0d0bf51362011c4d841a89fb8f5cfb16e0bf3
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-posix-acl.c
    M hw/9pfs/9p-xattr-user.c
    M hw/9pfs/9p-xattr.c
    M hw/9pfs/9p-xattr.h

  Log Message:
  -----------
  9pfs: local: lremovexattr: don't follow symlinks

The local_lremovexattr() callback is vulnerable to symlink attacks because
it calls lremovexattr() which follows symbolic links in all path elements
but the rightmost one.

This patch introduces a helper to emulate the non-existing fremovexattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to lremovexattr().

local_lremovexattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: df4938a6651b1f980018f9eaf86af43e6b9d7fed
      
https://github.com/qemu/qemu/commit/df4938a6651b1f980018f9eaf86af43e6b9d7fed
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: unlinkat: don't follow symlinks

The local_unlinkat() callback is vulnerable to symlink attacks because it
calls remove() which follows symbolic links in all path elements but the
rightmost one.

This patch converts local_unlinkat() to rely on opendir_nofollow() and
unlinkat() instead.

Most of the code is moved to a separate local_unlinkat_common() helper
which will be reused in a subsequent patch to fix the same issue in
local_remove().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: a0e640a87210b1e986bcd4e7f7de03beb3db0a4a
      
https://github.com/qemu/qemu/commit/a0e640a87210b1e986bcd4e7f7de03beb3db0a4a
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: remove: don't follow symlinks

The local_remove() callback is vulnerable to symlink attacks because it
calls:

(1) lstat() which follows symbolic links in all path elements but the
    rightmost one
(2) remove() which follows symbolic links in all path elements but the
    rightmost one

This patch converts local_remove() to rely on opendir_nofollow(),
fstatat(AT_SYMLINK_NOFOLLOW) to fix (1) and unlinkat() to fix (2).

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: a33eda0dd99e00faa3bacae43d19490bb9500e07
      
https://github.com/qemu/qemu/commit/a33eda0dd99e00faa3bacae43d19490bb9500e07
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: utimensat: don't follow symlinks

The local_utimensat() callback is vulnerable to symlink attacks because it
calls qemu_utimens()->utimensat(AT_SYMLINK_NOFOLLOW) which follows symbolic
links in all path elements but the rightmost one or qemu_utimens()->utimes()
which follows symbolic links for all path elements.

This patch converts local_utimensat() to rely on opendir_nofollow() and
utimensat(AT_SYMLINK_NOFOLLOW) directly instead of using qemu_utimens().
It is hence assumed that the OS supports utimensat(), i.e. has glibc 2.6
or higher and linux 2.6.22 or higher, which seems reasonable nowadays.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 31e51d1c15b35dc98b88a301812914b70a2b55dc
      
https://github.com/qemu/qemu/commit/31e51d1c15b35dc98b88a301812914b70a2b55dc
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: statfs: don't follow symlinks

The local_statfs() callback is vulnerable to symlink attacks because it
calls statfs() which follows symbolic links in all path elements.

This patch converts local_statfs() to rely on open_nofollow() and fstatfs()
instead.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: ac125d993b461d4dee4d6df4d93ac3f2eb959d1d
      
https://github.com/qemu/qemu/commit/ac125d993b461d4dee4d6df4d93ac3f2eb959d1d
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: truncate: don't follow symlinks

The local_truncate() callback is vulnerable to symlink attacks because
it calls truncate() which follows symbolic links in all path elements.

This patch converts local_truncate() to rely on open_nofollow() and
ftruncate() instead.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: bec1e9546e03b9e7f5152cf3e8c95cf8acff5e12
      
https://github.com/qemu/qemu/commit/bec1e9546e03b9e7f5152cf3e8c95cf8acff5e12
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: readlink: don't follow symlinks

The local_readlink() callback is vulnerable to symlink attacks because it
calls:

(1) open(O_NOFOLLOW) which follows symbolic links for all path elements but
    the rightmost one
(2) readlink() which follows symbolic links for all path elements but the
    rightmost one

This patch converts local_readlink() to rely on open_nofollow() to fix (1)
and opendir_nofollow(), readlinkat() to fix (2).

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: f9aef99b3e6df88036436b0d3dc3d504b9346c8c
      
https://github.com/qemu/qemu/commit/f9aef99b3e6df88036436b0d3dc3d504b9346c8c
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: lstat: don't follow symlinks

The local_lstat() callback is vulnerable to symlink attacks because it
calls:

(1) lstat() which follows symbolic links in all path elements but the
    rightmost one
(2) getxattr() which follows symbolic links in all path elements
(3) local_mapped_file_attr()->local_fopen()->openat(O_NOFOLLOW) which
    follows symbolic links in all path elements but the rightmost
    one

This patch converts local_lstat() to rely on opendir_nofollow() and
fstatat(AT_SYMLINK_NOFOLLOW) to fix (1), fgetxattrat_nofollow() to
fix (2).

A new local_fopenat() helper is introduced as a replacement to
local_fopen() to fix (3). No effort is made to factor out code
because local_fopen() will be dropped when all users have been
converted to call local_fopenat().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 99f2cf4b2dad7b37c69759deb0d0b19d3ec1a24a
      
https://github.com/qemu/qemu/commit/99f2cf4b2dad7b37c69759deb0d0b19d3ec1a24a
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: renameat: don't follow symlinks

The local_renameat() callback is currently a wrapper around local_rename()
which is vulnerable to symlink attacks.

This patch rewrites local_renameat() to have its own implementation, based
on local_opendir_nofollow() and renameat().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: d2767edec582558f1e6c52e1dd9370d62e2b30fc
      
https://github.com/qemu/qemu/commit/d2767edec582558f1e6c52e1dd9370d62e2b30fc
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: rename: use renameat

The local_rename() callback is vulnerable to symlink attacks because it
uses rename() which follows symbolic links in all path elements but the
rightmost one.

This patch simply transforms local_rename() into a wrapper around
local_renameat() which is symlink-attack safe.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 6dd4b1f1d026e478d9177b28169b377e212400f3
      
https://github.com/qemu/qemu/commit/6dd4b1f1d026e478d9177b28169b377e212400f3
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: improve error handling in link op

When using the mapped-file security model, we also have to create a link
for the metadata file if it exists. In case of failure, we should rollback.

That's what this patch does.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: ad0b46e6ac769b187cb4dcf0065675ef8a198a5e
      
https://github.com/qemu/qemu/commit/ad0b46e6ac769b187cb4dcf0065675ef8a198a5e
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: link: don't follow symlinks

The local_link() callback is vulnerable to symlink attacks because it calls:

(1) link() which follows symbolic links for all path elements but the
    rightmost one
(2) local_create_mapped_attr_dir()->mkdir() which follows symbolic links
    for all path elements but the rightmost one

This patch converts local_link() to rely on opendir_nofollow() and linkat()
to fix (1), mkdirat() to fix (2).

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: e3187a45dd02a7490f9191c16527dc28a4ba45b9
      
https://github.com/qemu/qemu/commit/e3187a45dd02a7490f9191c16527dc28a4ba45b9
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: chmod: don't follow symlinks

The local_chmod() callback is vulnerable to symlink attacks because it
calls:

(1) chmod() which follows symbolic links for all path elements
(2) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(3) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one

We would need fchmodat() to implement AT_SYMLINK_NOFOLLOW to fix (1). This
isn't the case on linux unfortunately: the kernel doesn't even have a flags
argument to the syscall :-\ It is impossible to fix it in userspace in
a race-free manner. This patch hence converts local_chmod() to rely on
open_nofollow() and fchmod(). This fixes the vulnerability but introduces
a limitation: the target file must readable and/or writable for the call
to openat() to succeed.

It introduces a local_set_xattrat() replacement to local_set_xattr()
based on fsetxattrat() to fix (2), and a local_set_mapped_file_attrat()
replacement to local_set_mapped_file_attr() based on local_fopenat()
and mkdirat() to fix (3). No effort is made to factor out code because
both local_set_xattr() and local_set_mapped_file_attr() will be dropped
when all users have been converted to use the "at" versions.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: d369f20763a857eac544a5289a046d0285a91df8
      
https://github.com/qemu/qemu/commit/d369f20763a857eac544a5289a046d0285a91df8
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: chown: don't follow symlinks

The local_chown() callback is vulnerable to symlink attacks because it
calls:

(1) lchown() which follows symbolic links for all path elements but the
    rightmost one
(2) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(3) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one

This patch converts local_chown() to rely on open_nofollow() and
fchownat() to fix (1), as well as local_set_xattrat() and
local_set_mapped_file_attrat() to fix (2) and (3) respectively.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 38771613ea6759f499645afd709aa422161eb27e
      
https://github.com/qemu/qemu/commit/38771613ea6759f499645afd709aa422161eb27e
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: symlink: don't follow symlinks

The local_symlink() callback is vulnerable to symlink attacks because it
calls:

(1) symlink() which follows symbolic links for all path elements but the
    rightmost one
(2) open(O_NOFOLLOW) which follows symbolic links for all path elements but
    the rightmost one
(3) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(4) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one

This patch converts local_symlink() to rely on opendir_nofollow() and
symlinkat() to fix (1), openat(O_NOFOLLOW) to fix (2), as well as
local_set_xattrat() and local_set_mapped_file_attrat() to fix (3) and
(4) respectively.

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: d815e7219036d6911fce12efe3e59906264c8536
      
https://github.com/qemu/qemu/commit/d815e7219036d6911fce12efe3e59906264c8536
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: mknod: don't follow symlinks

The local_mknod() callback is vulnerable to symlink attacks because it
calls:

(1) mknod() which follows symbolic links for all path elements but the
    rightmost one
(2) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(3) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one
(4) local_post_create_passthrough() which calls in turn lchown() and
    chmod(), both functions also following symbolic links

This patch converts local_mknod() to rely on opendir_nofollow() and
mknodat() to fix (1), as well as local_set_xattrat() and
local_set_mapped_file_attrat() to fix (2) and (3) respectively.

A new local_set_cred_passthrough() helper based on fchownat() and
fchmodat_nofollow() is introduced as a replacement to
local_post_create_passthrough() to fix (4).

The mapped and mapped-file security modes are supposed to be identical,
except for the place where credentials and file modes are stored. While
here, we also make that explicit by sharing the call to mknodat().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 3f3a16990b09e62d787bd2eb2dd51aafbe90019a
      
https://github.com/qemu/qemu/commit/3f3a16990b09e62d787bd2eb2dd51aafbe90019a
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: mkdir: don't follow symlinks

The local_mkdir() callback is vulnerable to symlink attacks because it
calls:

(1) mkdir() which follows symbolic links for all path elements but the
    rightmost one
(2) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(3) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one
(4) local_post_create_passthrough() which calls in turn lchown() and
    chmod(), both functions also following symbolic links

This patch converts local_mkdir() to rely on opendir_nofollow() and
mkdirat() to fix (1), as well as local_set_xattrat(),
local_set_mapped_file_attrat() and local_set_cred_passthrough() to
fix (2), (3) and (4) respectively.

The mapped and mapped-file security modes are supposed to be identical,
except for the place where credentials and file modes are stored. While
here, we also make that explicit by sharing the call to mkdirat().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: a565fea56546e254b7610305b07711f0a3bda0c7
      
https://github.com/qemu/qemu/commit/a565fea56546e254b7610305b07711f0a3bda0c7
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: open2: don't follow symlinks

The local_open2() callback is vulnerable to symlink attacks because it
calls:

(1) open() which follows symbolic links for all path elements but the
    rightmost one
(2) local_set_xattr()->setxattr() which follows symbolic links for all
    path elements
(3) local_set_mapped_file_attr() which calls in turn local_fopen() and
    mkdir(), both functions following symbolic links for all path
    elements but the rightmost one
(4) local_post_create_passthrough() which calls in turn lchown() and
    chmod(), both functions also following symbolic links

This patch converts local_open2() to rely on opendir_nofollow() and
mkdirat() to fix (1), as well as local_set_xattrat(),
local_set_mapped_file_attrat() and local_set_cred_passthrough() to
fix (2), (3) and (4) respectively. Since local_open2() already opens
a descriptor to the target file, local_set_cred_passthrough() is
modified to reuse it instead of opening a new one.

The mapped and mapped-file security modes are supposed to be identical,
except for the place where credentials and file modes are stored. While
here, we also make that explicit by sharing the call to openat().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: c23d5f1d5bc0e23aeb845b1af8f996f16783ce98
      
https://github.com/qemu/qemu/commit/c23d5f1d5bc0e23aeb845b1af8f996f16783ce98
  Author: Greg Kurz <address@hidden>
  Date:   2017-02-28 (Tue, 28 Feb 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: drop unused code

Now that the all callbacks have been converted to use "at" syscalls, we
can drop this code.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 7287e3556fdc56bfd0666a67d6b1d3ca9ce04083
      
https://github.com/qemu/qemu/commit/7287e3556fdc56bfd0666a67d6b1d3ca9ce04083
  Author: Peter Maydell <address@hidden>
  Date:   2017-03-01 (Wed, 01 Mar 2017)

  Changed paths:
    M hw/9pfs/9p-local.c
    A hw/9pfs/9p-local.h
    M hw/9pfs/9p-posix-acl.c
    A hw/9pfs/9p-util.c
    A hw/9pfs/9p-util.h
    M hw/9pfs/9p-xattr-user.c
    M hw/9pfs/9p-xattr.c
    M hw/9pfs/9p-xattr.h
    M hw/9pfs/Makefile.objs

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/gkurz/tags/cve-2016-9602-for-upstream' 
into staging

This pull request have all the fixes for CVE-2016-9602, so that it can
be easily picked up by downstreams, as suggested by Michel Tokarev.

# gpg: Signature made Tue 28 Feb 2017 10:21:32 GMT
# gpg:                using DSA key 0x02FC3AEB0101DBC2
# gpg: Good signature from "Greg Kurz <address@hidden>"
# gpg:                 aka "Greg Kurz <address@hidden>"
# gpg:                 aka "Greg Kurz <address@hidden>"
# gpg:                 aka "Gregory Kurz (Groug) <address@hidden>"
# gpg:                 aka "[jpeg image of size 3330]"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 2BD4 3B44 535E C0A7 9894  DBA2 02FC 3AEB 0101 DBC2

* remotes/gkurz/tags/cve-2016-9602-for-upstream: (28 commits)
  9pfs: local: drop unused code
  9pfs: local: open2: don't follow symlinks
  9pfs: local: mkdir: don't follow symlinks
  9pfs: local: mknod: don't follow symlinks
  9pfs: local: symlink: don't follow symlinks
  9pfs: local: chown: don't follow symlinks
  9pfs: local: chmod: don't follow symlinks
  9pfs: local: link: don't follow symlinks
  9pfs: local: improve error handling in link op
  9pfs: local: rename: use renameat
  9pfs: local: renameat: don't follow symlinks
  9pfs: local: lstat: don't follow symlinks
  9pfs: local: readlink: don't follow symlinks
  9pfs: local: truncate: don't follow symlinks
  9pfs: local: statfs: don't follow symlinks
  9pfs: local: utimensat: don't follow symlinks
  9pfs: local: remove: don't follow symlinks
  9pfs: local: unlinkat: don't follow symlinks
  9pfs: local: lremovexattr: don't follow symlinks
  9pfs: local: lsetxattr: don't follow symlinks
  ...

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/e3280ffbf59b...7287e3556fdc

reply via email to

[Prev in Thread] Current Thread [Next in Thread]