[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 23/29] 9pfs: local: chmod: don't follow symlinks
From: |
Greg Kurz |
Subject: |
[Qemu-devel] [PATCH 23/29] 9pfs: local: chmod: don't follow symlinks |
Date: |
Mon, 20 Feb 2017 15:42:19 +0100 |
User-agent: |
StGit/0.17.1-20-gc0b1b-dirty |
The local_chmod() callback is vulnerable to symlink attacks because it
calls:
(1) chmod() which follows symbolic links for all path elements
(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
This patch converts local_chmod() to rely on open_nofollow() and
fchmod() to fix (1).
It introduces a local_set_xattrat() replacement to local_set_xattr()
based on fsetxattrat() to fix (2), and a local_set_mapped_file_attrat()
replacement to local_set_mapped_file_attr() based on local_fopenat()
and mkdirat() to fix (3). No effort is made to factor out code because
both local_set_xattr() and local_set_mapped_file_attr() will be dropped
when all users have been converted to use the "at" versions.
This partly fixes CVE-2016-9602.
Signed-off-by: Greg Kurz <address@hidden>
---
hw/9pfs/9p-local.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 150 insertions(+), 13 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index fb536fdb3082..56d7731f7ce1 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -327,6 +327,87 @@ err_out:
return ret;
}
+static int local_set_mapped_file_attrat(int dirfd, const char *name,
+ FsCred *credp)
+{
+ FILE *fp;
+ int ret;
+ char buf[ATTR_MAX];
+ int uid = -1, gid = -1, mode = -1, rdev = -1;
+ int map_dirfd;
+
+ ret = mkdirat(dirfd, VIRTFS_META_DIR, 0700);
+ if (ret < 0 && errno != EEXIST) {
+ return -1;
+ }
+
+ map_dirfd = openat(dirfd, VIRTFS_META_DIR,
+ O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (map_dirfd == -1) {
+ return -1;
+ }
+
+ fp = local_fopenat(map_dirfd, name, "r");
+ if (!fp) {
+ if (errno == ENOENT) {
+ goto update_map_file;
+ } else {
+ close_preserve_errno(map_dirfd);
+ return -1;
+ }
+ }
+ memset(buf, 0, ATTR_MAX);
+ while (fgets(buf, ATTR_MAX, fp)) {
+ if (!strncmp(buf, "virtfs.uid", 10)) {
+ uid = atoi(buf + 11);
+ } else if (!strncmp(buf, "virtfs.gid", 10)) {
+ gid = atoi(buf + 11);
+ } else if (!strncmp(buf, "virtfs.mode", 11)) {
+ mode = atoi(buf + 12);
+ } else if (!strncmp(buf, "virtfs.rdev", 11)) {
+ rdev = atoi(buf + 12);
+ }
+ memset(buf, 0, ATTR_MAX);
+ }
+ fclose(fp);
+
+update_map_file:
+ fp = local_fopenat(map_dirfd, name, "w");
+ close_preserve_errno(map_dirfd);
+ if (!fp) {
+ return -1;
+ }
+
+ if (credp->fc_uid != -1) {
+ uid = credp->fc_uid;
+ }
+ if (credp->fc_gid != -1) {
+ gid = credp->fc_gid;
+ }
+ if (credp->fc_mode != -1) {
+ mode = credp->fc_mode;
+ }
+ if (credp->fc_rdev != -1) {
+ rdev = credp->fc_rdev;
+ }
+
+ if (uid != -1) {
+ fprintf(fp, "virtfs.uid=%d\n", uid);
+ }
+ if (gid != -1) {
+ fprintf(fp, "virtfs.gid=%d\n", gid);
+ }
+ if (mode != -1) {
+ fprintf(fp, "virtfs.mode=%d\n", mode);
+ }
+ if (rdev != -1) {
+ fprintf(fp, "virtfs.rdev=%d\n", rdev);
+ }
+ fclose(fp);
+
+ return 0;
+}
+
static int local_set_xattr(const char *path, FsCred *credp)
{
int err;
@@ -362,6 +443,45 @@ static int local_set_xattr(const char *path, FsCred *credp)
return 0;
}
+static int local_set_xattrat(int dirfd, const char *path, FsCred *credp)
+{
+ int err;
+
+ if (credp->fc_uid != -1) {
+ uint32_t tmp_uid = cpu_to_le32(credp->fc_uid);
+ err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.uid", &tmp_uid,
+ sizeof(uid_t), 0);
+ if (err) {
+ return err;
+ }
+ }
+ if (credp->fc_gid != -1) {
+ uint32_t tmp_gid = cpu_to_le32(credp->fc_gid);
+ err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.gid", &tmp_gid,
+ sizeof(gid_t), 0);
+ if (err) {
+ return err;
+ }
+ }
+ if (credp->fc_mode != -1) {
+ uint32_t tmp_mode = cpu_to_le32(credp->fc_mode);
+ err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.mode", &tmp_mode,
+ sizeof(mode_t), 0);
+ if (err) {
+ return err;
+ }
+ }
+ if (credp->fc_rdev != -1) {
+ uint64_t tmp_rdev = cpu_to_le64(credp->fc_rdev);
+ err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.rdev", &tmp_rdev,
+ sizeof(dev_t), 0);
+ if (err) {
+ return err;
+ }
+ }
+ return 0;
+}
+
static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
FsCred *credp)
{
@@ -553,21 +673,38 @@ static ssize_t local_pwritev(FsContext *ctx,
V9fsFidOpenState *fs,
static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
{
- char *buffer;
int ret = -1;
- char *path = fs_path->data;
+ int dirfd;
- if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
- buffer = rpath(fs_ctx, path);
- ret = local_set_xattr(buffer, credp);
- g_free(buffer);
- } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
- return local_set_mapped_file_attr(fs_ctx, path, credp);
- } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
- (fs_ctx->export_flags & V9FS_SM_NONE)) {
- buffer = rpath(fs_ctx, path);
- ret = chmod(buffer, credp->fc_mode);
- g_free(buffer);
+ if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
+ fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
+ char *dirpath, *name;
+
+ dirpath = g_path_get_dirname(fs_path->data);
+ dirfd = local_opendir_nofollow(fs_ctx, dirpath);
+ g_free(dirpath);
+ if (dirfd == -1) {
+ return -1;
+ }
+
+ name = g_path_get_basename(fs_path->data);
+ if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
+ ret = local_set_xattrat(dirfd, name, credp);
+ } else {
+ ret = local_set_mapped_file_attrat(dirfd, name, credp);
+ }
+ g_free(name);
+ close_preserve_errno(dirfd);
+ } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
+ fs_ctx->export_flags & V9FS_SM_NONE) {
+ int fd;
+
+ fd = local_open_nofollow(fs_ctx, fs_path->data, O_RDONLY, 0);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = fchmod(fd, credp->fc_mode);
+ close_preserve_errno(fd);
}
return ret;
}
- [Qemu-devel] [PATCH 18/29] 9pfs: local: lstat: don't follow symlinks, (continued)
- [Qemu-devel] [PATCH 18/29] 9pfs: local: lstat: don't follow symlinks, Greg Kurz, 2017/02/20
- [Qemu-devel] [PATCH 19/29] 9pfs: local: renameat: don't follow symlinks, Greg Kurz, 2017/02/20
- [Qemu-devel] [PATCH 20/29] 9pfs: local: rename: use renameat, Greg Kurz, 2017/02/20
- [Qemu-devel] [PATCH 21/29] 9pfs: local: improve error handling in link op, Greg Kurz, 2017/02/20
- [Qemu-devel] [PATCH 22/29] 9pfs: local: link: don't follow symlinks, Greg Kurz, 2017/02/20
- [Qemu-devel] [PATCH 23/29] 9pfs: local: chmod: don't follow symlinks,
Greg Kurz <=
[Qemu-devel] [PATCH 24/29] 9pfs: local: chown: don't follow symlinks, Greg Kurz, 2017/02/20
[Qemu-devel] [PATCH 25/29] 9pfs: local: symlink: don't follow symlinks, Greg Kurz, 2017/02/20
[Qemu-devel] [PATCH 26/29] 9pfs: local: mknod: don't follow symlinks, Greg Kurz, 2017/02/20