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 12/28] 9pfs: local: remove: don't follow symlinks
Date: Tue, 28 Feb 2017 11:30:24 +0100 [thread overview]
Message-ID: <1488277840-18608-13-git-send-email-groug@kaod.org> (raw)
In-Reply-To: <1488277840-18608-1-git-send-email-groug@kaod.org>
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 <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/9pfs/9p-local.c | 64 ++++++++++++++++++------------------------------------
1 file changed, 21 insertions(+), 43 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 04de511765d8..8fb79e44b5f3 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1021,54 +1021,32 @@ err_out:
static int local_remove(FsContext *ctx, const char *path)
{
- int err;
struct stat stbuf;
- char *buffer;
+ char *dirpath = g_path_get_dirname(path);
+ char *name = g_path_get_basename(path);
+ int flags = 0;
+ int dirfd;
+ int err = -1;
- if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
- buffer = rpath(ctx, path);
- err = lstat(buffer, &stbuf);
- g_free(buffer);
- if (err) {
- goto err_out;
- }
- /*
- * If directory remove .virtfs_metadata contained in the
- * directory
- */
- if (S_ISDIR(stbuf.st_mode)) {
- buffer = g_strdup_printf("%s/%s/%s", ctx->fs_root,
- path, VIRTFS_META_DIR);
- err = remove(buffer);
- g_free(buffer);
- if (err < 0 && errno != ENOENT) {
- /*
- * We didn't had the .virtfs_metadata file. May be file created
- * in non-mapped mode ?. Ignore ENOENT.
- */
- goto err_out;
- }
- }
- /*
- * Now remove the name from parent directory
- * .virtfs_metadata directory
- */
- buffer = local_mapped_attr_path(ctx, path);
- err = remove(buffer);
- g_free(buffer);
- if (err < 0 && errno != ENOENT) {
- /*
- * We didn't had the .virtfs_metadata file. May be file created
- * in non-mapped mode ?. Ignore ENOENT.
- */
- goto err_out;
- }
+ dirfd = local_opendir_nofollow(ctx, dirpath);
+ if (dirfd) {
+ goto out;
}
- buffer = rpath(ctx, path);
- err = remove(buffer);
- g_free(buffer);
+ if (fstatat(dirfd, path, &stbuf, AT_SYMLINK_NOFOLLOW) < 0) {
+ goto err_out;
+ }
+
+ if (S_ISDIR(stbuf.st_mode)) {
+ flags |= AT_REMOVEDIR;
+ }
+
+ err = local_unlinkat_common(ctx, dirfd, name, flags);
err_out:
+ close_preserve_errno(dirfd);
+out:
+ g_free(name);
+ g_free(dirpath);
return err;
}
--
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 ` Greg Kurz [this message]
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 ` [Qemu-devel] [PULL 16/28] 9pfs: local: readlink: " Greg Kurz
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-13-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.