aboutsummaryrefslogtreecommitdiff
path: root/hw/9pfs/9p-local.c
diff options
context:
space:
mode:
authorGreg Kurz <groug@kaod.org>2017-02-26 23:43:08 +0100
committerGreg Kurz <groug@kaod.org>2017-02-28 11:21:15 +0100
commita0e640a87210b1e986bcd4e7f7de03beb3db0a4a (patch)
tree47fe2cc4eb1d19b7b63b6ebb7e67f5bf299868f8 /hw/9pfs/9p-local.c
parentdf4938a6651b1f980018f9eaf86af43e6b9d7fed (diff)
downloadqemu-a0e640a87210b1e986bcd4e7f7de03beb3db0a4a.zip
qemu-a0e640a87210b1e986bcd4e7f7de03beb3db0a4a.tar.gz
qemu-a0e640a87210b1e986bcd4e7f7de03beb3db0a4a.tar.bz2
9pfs: local: remove: don't follow symlinks
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>
Diffstat (limited to 'hw/9pfs/9p-local.c')
-rw-r--r--hw/9pfs/9p-local.c64
1 files changed, 21 insertions, 43 deletions
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 04de511..8fb79e4 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;
}