[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path
From: |
Aneesh Kumar K.V |
Subject: |
Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path |
Date: |
Thu, 11 Aug 2016 12:01:46 +0530 |
User-agent: |
Notmuch/0.21 (https://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) |
P J P <address@hidden> writes:
> From: Prasad J Pandit <address@hidden>
>
> At various places in 9pfs back-end, it creates full path by
> concatenating two path strings. It could lead to a path
> traversal issue if one of the parameter was a relative path.
> Add check to avoid it.
>
> Reported-by: Felix Wilhelm <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
I am not sure relative path names need to be completely disallowed. What
we need is to disallow the access outside export path. virtfs-proxy was
done primarily to handle such things. It does a chroot to the export
path.
> ---
> hw/9pfs/9p-local.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 3f271fc..c20331a 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -493,6 +493,9 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> *dir_path,
> char *buffer = NULL;
>
> v9fs_string_init(&fullname);
> + if (strstr(name, "../")) {
> + return err;
> + }
> v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
> path = fullname.data;
>
> @@ -554,6 +557,9 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath
> *dir_path,
> char *buffer = NULL;
>
> v9fs_string_init(&fullname);
> + if (strstr(name, "../")) {
> + return err;
> + }
> v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
> path = fullname.data;
>
> @@ -663,6 +669,9 @@ static int local_open2(FsContext *fs_ctx, V9fsPath
> *dir_path, const char *name,
> flags |= O_NOFOLLOW;
>
> v9fs_string_init(&fullname);
> + if (strstr(name, "../")) {
> + return err;
> + }
> v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
> path = fullname.data;
>
> @@ -734,6 +743,9 @@ static int local_symlink(FsContext *fs_ctx, const char
> *oldpath,
> char *buffer = NULL;
>
> v9fs_string_init(&fullname);
> + if (strstr(name, "../")) {
> + return err;
> + }
> v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
> newpath = fullname.data;
>
> @@ -830,11 +842,14 @@ out:
> static int local_link(FsContext *ctx, V9fsPath *oldpath,
> V9fsPath *dirpath, const char *name)
> {
> - int ret;
> + int ret = -1;
> V9fsString newpath;
> char *buffer, *buffer1;
>
> v9fs_string_init(&newpath);
> + if (strstr(name, "../")) {
> + return ret;
> + }
> v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
>
> buffer = rpath(ctx, oldpath->data);
> @@ -1059,6 +1074,9 @@ static int local_lremovexattr(FsContext *ctx, V9fsPath
> *fs_path,
> static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
> const char *name, V9fsPath *target)
> {
> + if (strstr(name, "../")) {
> + return -1;
> + }
> if (dir_path) {
> v9fs_string_sprintf((V9fsString *)target, "%s/%s",
> dir_path->data, name);
> @@ -1074,12 +1092,15 @@ static int local_renameat(FsContext *ctx, V9fsPath
> *olddir,
> const char *old_name, V9fsPath *newdir,
> const char *new_name)
> {
> - int ret;
> + int ret = -1;
> V9fsString old_full_name, new_full_name;
>
> v9fs_string_init(&old_full_name);
> v9fs_string_init(&new_full_name);
>
> + if (strstr(old_name, "../") || strstr(new_name, "../")) {
> + return ret;
> + }
> v9fs_string_sprintf(&old_full_name, "%s/%s", olddir->data, old_name);
> v9fs_string_sprintf(&new_full_name, "%s/%s", newdir->data, new_name);
>
> @@ -1092,12 +1113,14 @@ static int local_renameat(FsContext *ctx, V9fsPath
> *olddir,
> static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
> const char *name, int flags)
> {
> - int ret;
> + int ret = -1;
> V9fsString fullname;
> char *buffer;
>
> v9fs_string_init(&fullname);
> -
> + if (strstr(name, "../")) {
> + return ret;
> + }
> v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name);
> if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> if (flags == AT_REMOVEDIR) {
> --
> 2.5.5
- [Qemu-devel] [PATCH] 9pfs: add check for relative path, P J P, 2016/08/11
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, no-reply, 2016/08/11
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path,
Aneesh Kumar K.V <=
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/18
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, P J P, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Michael S. Tsirkin, 2016/08/22