[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 30/31] 9pfs: local: open2: don't follow symlinks
From: |
Greg Kurz |
Subject: |
[Qemu-devel] [PULL 30/31] 9pfs: local: open2: don't follow symlinks |
Date: |
Tue, 28 Feb 2017 00:00:20 +0100 |
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>
---
hw/9pfs/9p-local.c | 56 ++++++++++++++++++------------------------------------
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 572eb5886e04..0b8bbf31a12d 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -887,62 +887,45 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
int flags, FsCred *credp, V9fsFidOpenState *fs)
{
- char *path;
int fd = -1;
int err = -1;
- int serrno = 0;
- V9fsString fullname;
- char *buffer = NULL;
+ int dirfd;
/*
* Mark all the open to not follow symlinks
*/
flags |= O_NOFOLLOW;
- v9fs_string_init(&fullname);
- v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
- path = fullname.data;
+ dirfd = local_opendir_nofollow(fs_ctx, dir_path->data);
+ if (dirfd == -1) {
+ return -1;
+ }
/* Determine the security model */
- if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
- buffer = rpath(fs_ctx, path);
- fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
+ if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
+ fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
+ fd = openat_file(dirfd, name, flags, SM_LOCAL_MODE_BITS);
if (fd == -1) {
- err = fd;
goto out;
}
credp->fc_mode = credp->fc_mode|S_IFREG;
- /* Set cleint credentials in xattr */
- err = local_set_xattr(buffer, credp);
- if (err == -1) {
- serrno = errno;
- goto err_end;
- }
- } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
- buffer = rpath(fs_ctx, path);
- fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
- if (fd == -1) {
- err = fd;
- goto out;
+ if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
+ /* Set cleint credentials in xattr */
+ err = local_set_xattrat(dirfd, name, credp);
+ } else {
+ err = local_set_mapped_file_attrat(dirfd, name, credp);
}
- credp->fc_mode = credp->fc_mode|S_IFREG;
- /* Set client credentials in .virtfs_metadata directory files */
- err = local_set_mapped_file_attr(fs_ctx, path, credp);
if (err == -1) {
- serrno = errno;
goto err_end;
}
} else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
(fs_ctx->export_flags & V9FS_SM_NONE)) {
- buffer = rpath(fs_ctx, path);
- fd = open(buffer, flags, credp->fc_mode);
+ fd = openat_file(dirfd, name, flags, credp->fc_mode);
if (fd == -1) {
- err = fd;
goto out;
}
- err = local_post_create_passthrough(fs_ctx, path, credp);
+ err = local_set_cred_passthrough(fs_ctx, dirfd, name, credp);
if (err == -1) {
- serrno = errno;
goto err_end;
}
}
@@ -951,12 +934,11 @@ static int local_open2(FsContext *fs_ctx, V9fsPath
*dir_path, const char *name,
goto out;
err_end:
- close(fd);
- remove(buffer);
- errno = serrno;
+ unlinkat_preserve_errno(dirfd, name,
+ flags & O_DIRECTORY ? AT_REMOVEDIR : 0);
+ close_preserve_errno(fd);
out:
- g_free(buffer);
- v9fs_string_free(&fullname);
+ close_preserve_errno(dirfd);
return err;
}
--
2.7.4
- [Qemu-devel] [PULL 04/31] 9pfs: local: move xattr security ops to 9p-xattr.c, (continued)
- [Qemu-devel] [PULL 04/31] 9pfs: local: move xattr security ops to 9p-xattr.c, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 14/31] 9pfs: local: unlinkat: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 17/31] 9pfs: local: statfs: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 19/31] 9pfs: local: readlink: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 20/31] 9pfs: local: lstat: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 23/31] 9pfs: local: improve error handling in link op, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 26/31] 9pfs: local: chown: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 18/31] 9pfs: local: truncate: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 22/31] 9pfs: local: rename: use renameat, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 09/31] 9pfs: local: open/opendir: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 30/31] 9pfs: local: open2: don't follow symlinks,
Greg Kurz <=
- [Qemu-devel] [PULL 29/31] 9pfs: local: mkdir: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 31/31] 9pfs: local: drop unused code, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 28/31] 9pfs: local: mknod: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 24/31] 9pfs: local: link: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 27/31] 9pfs: local: symlink: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 25/31] 9pfs: local: chmod: don't follow symlinks, Greg Kurz, 2017/02/27
- Re: [Qemu-devel] [PULL 00/31] 9p patches 2017-02-27 for 2.9 soft freeze, no-reply, 2017/02/27
- Re: [Qemu-devel] [PULL 00/31] 9p patches 2017-02-27 for 2.9 soft freeze, no-reply, 2017/02/27
- Re: [Qemu-devel] [PULL 00/31] 9p patches 2017-02-27 for 2.9 soft freeze, Greg Kurz, 2017/02/27