From: Greg Kurz <groug@kaod.org>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
Greg Kurz <groug@kaod.org>
Subject: [Qemu-devel] [PULL 16/28] 9pfs: local: readlink: don't follow symlinks
Date: Tue, 28 Feb 2017 11:30:28 +0100 [thread overview]
Message-ID: <1488277840-18608-17-git-send-email-groug@kaod.org> (raw)
In-Reply-To: <1488277840-18608-1-git-send-email-groug@kaod.org>
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 <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/9pfs/9p-local.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 1a3dfd774012..e373cca3b78d 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -340,27 +340,35 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
char *buf, size_t bufsz)
{
ssize_t tsize = -1;
- char *buffer;
- char *path = fs_path->data;
if ((fs_ctx->export_flags & V9FS_SM_MAPPED) ||
(fs_ctx->export_flags & V9FS_SM_MAPPED_FILE)) {
int fd;
- buffer = rpath(fs_ctx, path);
- fd = open(buffer, O_RDONLY | O_NOFOLLOW);
- g_free(buffer);
+
+ fd = local_open_nofollow(fs_ctx, fs_path->data, O_RDONLY, 0);
if (fd == -1) {
return -1;
}
do {
tsize = read(fd, (void *)buf, bufsz);
} while (tsize == -1 && errno == EINTR);
- close(fd);
+ close_preserve_errno(fd);
} else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
(fs_ctx->export_flags & V9FS_SM_NONE)) {
- buffer = rpath(fs_ctx, path);
- tsize = readlink(buffer, buf, bufsz);
- g_free(buffer);
+ char *dirpath = g_path_get_dirname(fs_path->data);
+ char *name = g_path_get_basename(fs_path->data);
+ int dirfd;
+
+ dirfd = local_opendir_nofollow(fs_ctx, dirpath);
+ if (dirfd == -1) {
+ goto out;
+ }
+
+ tsize = readlinkat(dirfd, name, buf, bufsz);
+ close_preserve_errno(dirfd);
+ out:
+ g_free(name);
+ g_free(dirpath);
}
return tsize;
}
--
2.7.4
next prev parent reply other threads:[~2017-02-28 10:31 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-28 10:30 [Qemu-devel] [PULL 00/28] 9p CVE-2016-9602 fixes 2017-02-28 for 2.9 soft freeze Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 01/28] 9pfs: local: move xattr security ops to 9p-xattr.c Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 02/28] 9pfs: remove side-effects in local_init() Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 03/28] 9pfs: remove side-effects in local_open() and local_opendir() Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 04/28] 9pfs: introduce relative_openat_nofollow() helper Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 05/28] 9pfs: local: keep a file descriptor on the shared folder Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 06/28] 9pfs: local: open/opendir: don't follow symlinks Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 07/28] 9pfs: local: lgetxattr: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 08/28] 9pfs: local: llistxattr: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 09/28] 9pfs: local: lsetxattr: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 10/28] 9pfs: local: lremovexattr: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 11/28] 9pfs: local: unlinkat: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 12/28] 9pfs: local: remove: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 13/28] 9pfs: local: utimensat: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 14/28] 9pfs: local: statfs: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 15/28] 9pfs: local: truncate: " Greg Kurz
2017-02-28 10:30 ` Greg Kurz [this message]
2017-02-28 10:30 ` [Qemu-devel] [PULL 17/28] 9pfs: local: lstat: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 18/28] 9pfs: local: renameat: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 19/28] 9pfs: local: rename: use renameat Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 20/28] 9pfs: local: improve error handling in link op Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 21/28] 9pfs: local: link: don't follow symlinks Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 22/28] 9pfs: local: chmod: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 23/28] 9pfs: local: chown: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 24/28] 9pfs: local: symlink: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 25/28] 9pfs: local: mknod: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 26/28] 9pfs: local: mkdir: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 27/28] 9pfs: local: open2: " Greg Kurz
2017-02-28 10:30 ` [Qemu-devel] [PULL 28/28] 9pfs: local: drop unused code Greg Kurz
2017-02-28 14:02 ` [Qemu-devel] [PULL 00/28] 9p CVE-2016-9602 fixes 2017-02-28 for 2.9 soft freeze Michael Tokarev
2017-02-28 14:22 ` Greg Kurz
2017-02-28 14:55 ` Michael Tokarev
2017-02-28 15:11 ` Greg Kurz
2017-02-28 16:01 ` Daniel P. Berrange
2017-02-28 16:09 ` Pranith Kumar
2017-03-01 14:33 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1488277840-18608-17-git-send-email-groug@kaod.org \
--to=groug@kaod.org \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).