The local_llistxattr() callback is vulnerable to symlink attacks because it calls llistxattr() which follows symbolic links in all path elements but the rightmost one.
This patch converts local_llistxattr() to rely on opendir_nofollow() and flistxattrat_nofollow() instead. This partly fixes CVE-2016-9602. Signed-off-by: Greg Kurz <gr...@kaod.org> --- hw/9pfs/9p-xattr.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c index 4c3c0046bd47..803d4bbbc50b 100644 --- a/hw/9pfs/9p-xattr.c +++ b/hw/9pfs/9p-xattr.c @@ -216,6 +216,11 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path, return name_size; } +static ssize_t flistxattrat(int dirfd, const char *path, char *list, + size_t size) +{ + return do_xattrat_op(XATTRAT_OP_LIST, dirfd, path, NULL, list, size, 0); +} /* * Get the list and pass to each layer to find out whether @@ -225,24 +230,37 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path, void *value, size_t vsize) { ssize_t size = 0; - char *buffer; void *ovalue = value; XattrOperations *xops; char *orig_value, *orig_value_start; ssize_t xattr_len, parsed_len = 0, attr_len; + char *dirpath, *name; + int dirfd; /* Get the actual len */ - buffer = rpath(ctx, path); - xattr_len = llistxattr(buffer, value, 0); + dirpath = g_path_get_dirname(path); + dirfd = local_opendir_nofollow(ctx, dirpath); + g_free(dirpath); + if (dirfd == -1) { + return -1; + } + + name = g_path_get_basename(path); + xattr_len = flistxattrat(dirfd, name, value, 0); if (xattr_len <= 0) { - g_free(buffer); + g_free(name); + close_preserve_errno(dirfd); return xattr_len; } /* Now fetch the xattr and find the actual size */ orig_value = g_malloc(xattr_len); - xattr_len = llistxattr(buffer, orig_value, xattr_len); - g_free(buffer); + xattr_len = flistxattrat(dirfd, name, orig_value, xattr_len); + g_free(name); + close_preserve_errno(dirfd); + if (xattr_len < 0) { + return -1; + } /* store the orig pointer */ orig_value_start = orig_value;