All of lore.kernel.org
 help / color / mirror / Atom feed
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 06/28] 9pfs: local: open/opendir: don't follow symlinks
Date: Tue, 28 Feb 2017 11:30:18 +0100	[thread overview]
Message-ID: <1488277840-18608-7-git-send-email-groug@kaod.org> (raw)
In-Reply-To: <1488277840-18608-1-git-send-email-groug@kaod.org>

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 <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/9pfs/9p-local.c | 37 +++++++++++++++++++++++++++----------
 hw/9pfs/9p-local.h | 20 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 10 deletions(-)
 create mode 100644 hw/9pfs/9p-local.h

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index be6be615149b..2c491af623f9 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "9p.h"
+#include "9p-local.h"
 #include "9p-xattr.h"
 #include "9p-util.h"
 #include "fsdev/qemu-fsdev.h"   /* local_ops */
@@ -48,6 +49,24 @@ typedef struct {
     int mountfd;
 } LocalData;
 
+int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags,
+                        mode_t mode)
+{
+    LocalData *data = fs_ctx->private;
+
+    /* All paths are relative to the path data->mountfd points to */
+    while (*path == '/') {
+        path++;
+    }
+
+    return relative_openat_nofollow(data->mountfd, path, flags, mode);
+}
+
+int local_opendir_nofollow(FsContext *fs_ctx, const char *path)
+{
+    return local_open_nofollow(fs_ctx, path, O_DIRECTORY | O_RDONLY, 0);
+}
+
 #define VIRTFS_META_DIR ".virtfs_metadata"
 
 static char *local_mapped_attr_path(FsContext *ctx, const char *path)
@@ -359,13 +378,9 @@ static int local_closedir(FsContext *ctx, V9fsFidOpenState *fs)
 static int local_open(FsContext *ctx, V9fsPath *fs_path,
                       int flags, V9fsFidOpenState *fs)
 {
-    char *buffer;
-    char *path = fs_path->data;
     int fd;
 
-    buffer = rpath(ctx, path);
-    fd = open(buffer, flags | O_NOFOLLOW);
-    g_free(buffer);
+    fd = local_open_nofollow(ctx, fs_path->data, flags, 0);
     if (fd == -1) {
         return -1;
     }
@@ -376,13 +391,15 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path,
 static int local_opendir(FsContext *ctx,
                          V9fsPath *fs_path, V9fsFidOpenState *fs)
 {
-    char *buffer;
-    char *path = fs_path->data;
+    int dirfd;
     DIR *stream;
 
-    buffer = rpath(ctx, path);
-    stream = opendir(buffer);
-    g_free(buffer);
+    dirfd = local_opendir_nofollow(ctx, fs_path->data);
+    if (dirfd == -1) {
+        return -1;
+    }
+
+    stream = fdopendir(dirfd);
     if (!stream) {
         return -1;
     }
diff --git a/hw/9pfs/9p-local.h b/hw/9pfs/9p-local.h
new file mode 100644
index 000000000000..32c72749d9df
--- /dev/null
+++ b/hw/9pfs/9p-local.h
@@ -0,0 +1,20 @@
+/*
+ * 9p local backend utilities
+ *
+ * Copyright IBM, Corp. 2017
+ *
+ * Authors:
+ *  Greg Kurz <groug@kaod.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_9P_LOCAL_H
+#define QEMU_9P_LOCAL_H
+
+int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags,
+                        mode_t mode);
+int local_opendir_nofollow(FsContext *fs_ctx, const char *path);
+
+#endif
-- 
2.7.4

  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 ` Greg Kurz [this message]
2017-02-28 10:30 ` [Qemu-devel] [PULL 07/28] 9pfs: local: lgetxattr: don't follow symlinks 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 ` [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-7-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.