9pfs: local: lstat: don't follow symlinks
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 <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index e373cca..b810c64 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -111,17 +111,49 @@
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 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 @@
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;
}
