[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 20/31] 9pfs: local: lstat: don't follow symlinks
From: |
Greg Kurz |
Subject: |
[Qemu-devel] [PULL 20/31] 9pfs: local: lstat: don't follow symlinks |
Date: |
Tue, 28 Feb 2017 00:00:10 +0100 |
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>
---
hw/9pfs/9p-local.c | 78 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 61 insertions(+), 17 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index add1b2e83dbe..ee3c1bd0a2be 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -111,17 +111,49 @@ static FILE *local_fopen(const char *path, const char
*mode)
return fp;
}
+static FILE *local_fopenat(int dirfd, const char *name, const char *mode)
+{
+ int fd, o_mode = 0;
+ FILE *fp;
+ int flags;
+ /*
+ * only supports two modes
+ */
+ if (mode[0] == 'r') {
+ flags = O_RDONLY;
+ } else if (mode[0] == 'w') {
+ flags = O_WRONLY | O_TRUNC | O_CREAT;
+ o_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+ } else {
+ return NULL;
+ }
+ fd = openat_file(dirfd, name, flags, o_mode);
+ if (fd == -1) {
+ return NULL;
+ }
+ fp = fdopen(fd, mode);
+ if (!fp) {
+ close(fd);
+ }
+ return fp;
+}
+
#define ATTR_MAX 100
-static void local_mapped_file_attr(FsContext *ctx, const char *path,
+static void local_mapped_file_attr(int dirfd, const char *name,
struct stat *stbuf)
{
FILE *fp;
char buf[ATTR_MAX];
- char *attr_path;
+ int map_dirfd;
- attr_path = local_mapped_attr_path(ctx, path);
- fp = local_fopen(attr_path, "r");
- g_free(attr_path);
+ map_dirfd = openat(dirfd, VIRTFS_META_DIR,
+ O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (map_dirfd == -1) {
+ return;
+ }
+
+ fp = local_fopenat(map_dirfd, name, "r");
+ close_preserve_errno(map_dirfd);
if (!fp) {
return;
}
@@ -143,12 +175,17 @@ static void local_mapped_file_attr(FsContext *ctx, const
char *path,
static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat
*stbuf)
{
- int err;
- char *buffer;
- char *path = fs_path->data;
+ int err = -1;
+ char *dirpath = g_path_get_dirname(fs_path->data);
+ char *name = g_path_get_basename(fs_path->data);
+ int dirfd;
- buffer = rpath(fs_ctx, path);
- err = lstat(buffer, stbuf);
+ dirfd = local_opendir_nofollow(fs_ctx, dirpath);
+ if (dirfd == -1) {
+ goto out;
+ }
+
+ err = fstatat(dirfd, name, stbuf, AT_SYMLINK_NOFOLLOW);
if (err) {
goto err_out;
}
@@ -158,25 +195,32 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath
*fs_path, struct stat *stbuf)
gid_t tmp_gid;
mode_t tmp_mode;
dev_t tmp_dev;
- if (getxattr(buffer, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+
+ if (fgetxattrat_nofollow(dirfd, name, "user.virtfs.uid", &tmp_uid,
+ sizeof(uid_t)) > 0) {
stbuf->st_uid = le32_to_cpu(tmp_uid);
}
- if (getxattr(buffer, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+ if (fgetxattrat_nofollow(dirfd, name, "user.virtfs.gid", &tmp_gid,
+ sizeof(gid_t)) > 0) {
stbuf->st_gid = le32_to_cpu(tmp_gid);
}
- if (getxattr(buffer, "user.virtfs.mode",
- &tmp_mode, sizeof(mode_t)) > 0) {
+ if (fgetxattrat_nofollow(dirfd, name, "user.virtfs.mode", &tmp_mode,
+ sizeof(mode_t)) > 0) {
stbuf->st_mode = le32_to_cpu(tmp_mode);
}
- if (getxattr(buffer, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0)
{
+ if (fgetxattrat_nofollow(dirfd, name, "user.virtfs.rdev", &tmp_dev,
+ sizeof(dev_t)) > 0) {
stbuf->st_rdev = le64_to_cpu(tmp_dev);
}
} else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
- local_mapped_file_attr(fs_ctx, path, stbuf);
+ local_mapped_file_attr(dirfd, name, stbuf);
}
err_out:
- g_free(buffer);
+ close_preserve_errno(dirfd);
+out:
+ g_free(name);
+ g_free(dirpath);
return err;
}
--
2.7.4
- [Qemu-devel] [PULL 02/31] fsdev: add IO throttle support to fsdev devices, (continued)
- [Qemu-devel] [PULL 02/31] fsdev: add IO throttle support to fsdev devices, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 12/31] 9pfs: local: lsetxattr: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 15/31] 9pfs: local: remove: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 03/31] throttle: factor out duplicate code, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 21/31] 9pfs: local: renameat: don't follow symlinks, Greg Kurz, 2017/02/27
- [Qemu-devel] [PULL 16/31] 9pfs: local: utimensat: don't follow symlinks, Greg Kurz, 2017/02/27
- [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 <=
- [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, 2017/02/27
- [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