2021-09-17 19:49 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
> Because of "..", files outside the share directory
> could be accessed. To prevent this, normalize
> the given path and remove all "." and ".."
> components.
>
> Signed-off-by: Hyunchul Lee <hyc.lee@xxxxxxxxx>
> Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
> ---
> fs/ksmbd/misc.c | 79 ++++++++++++++++++++++++++++++++++++++++------
> fs/ksmbd/misc.h | 3 +-
> fs/ksmbd/smb2pdu.c | 14 +++++---
> 3 files changed, 80 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ksmbd/misc.c b/fs/ksmbd/misc.c
> index 0b307ca28a19..d599cb686415 100644
> --- a/fs/ksmbd/misc.c
> +++ b/fs/ksmbd/misc.c
> @@ -191,19 +191,80 @@ int get_nlink(struct kstat *st)
> return nlink;
> }
>
> -void ksmbd_conv_path_to_unix(char *path)
> +char *ksmbd_conv_path_to_unix(char *path)
> {
> + size_t path_len, remain_path_len, out_path_len;
> + char *out_path, *out_next;
> + int i, pre_dotdot_cnt = 0, slash_cnt = 0;
> + bool is_last;
> +
> strreplace(path, '\\', '/');
> -}
> + path_len = strlen(path);
> + remain_path_len = path_len;
> + if (path_len == 0)
> + return ERR_PTR(-EINVAL);
>
> -void ksmbd_strip_last_slash(char *path)
> -{
> - int len = strlen(path);
> + out_path = kzalloc(path_len + 2, GFP_KERNEL);
> + if (!out_path)
> + return ERR_PTR(-ENOMEM);
> + out_path_len = 0;
> + out_next = out_path;
> +
> + do {
> + char *name = path + path_len - remain_path_len;
> + char *next = strchrnul(name, '/');
> + size_t name_len = next - name;
> +
> + is_last = !next[0];
> + if (name_len == 2 && name[0] == '.' && name[1] == '.') {
> + pre_dotdot_cnt++;
> + /* handle the case that path ends with "/.." */
> + if (is_last)
> + goto follow_dotdot;
> + } else {
> + if (pre_dotdot_cnt) {
> +follow_dotdot:
> + slash_cnt = 0;
> + for (i = out_path_len - 1; i >= 0; i--) {
> + if (out_path[i] == '/') {
> + slash_cnt++;
> + if (slash_cnt ==
> + pre_dotdot_cnt + 1)
> + break;
> + }
checkpatch.pl warn:
WARNING: Too many leading tabs - consider code refactoring
#70: FILE: fs/ksmbd/misc.c:231:
+ if (slash_cnt ==
total: 0 errors, 1 warnings, 125 lines checked
updated like this.
if (out_path[i] == '/' &&
++slash_cnt == pre_dotdot_cnt + 1)
break;
Thanks.
> + }
> +
> + if (i < 0 &&
> + slash_cnt != pre_dotdot_cnt) {
> + kfree(out_path);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + out_next = &out_path[i+1];
> + *out_next = '\0';
> + out_path_len = i + 1;
>
> - while (len && path[len - 1] == '/') {
> - path[len - 1] = '\0';
> - len--;
> - }
> + }
> +
> + if (name_len != 0 &&
> + !(name_len == 1 && name[0] == '.') &&
> + !(name_len == 2 && name[0] == '.' && name[1] == '.')) {
> + next[0] = '\0';
> + sprintf(out_next, "%s/", name);
> + out_next += name_len + 1;
> + out_path_len += name_len + 1;
> + next[0] = '/';
> + }
> + pre_dotdot_cnt = 0;
> + }
> +
> + remain_path_len -= name_len + 1;
> + } while (!is_last);
> +
> + if (out_path_len > 0)
> + out_path[out_path_len-1] = '\0';
> + path[path_len] = '\0';
> + return out_path;
> }
>
> void ksmbd_conv_path_to_windows(char *path)
> diff --git a/fs/ksmbd/misc.h b/fs/ksmbd/misc.h
> index af8717d4d85b..b7b10139ada2 100644
> --- a/fs/ksmbd/misc.h
> +++ b/fs/ksmbd/misc.h
> @@ -16,8 +16,7 @@ int ksmbd_validate_filename(char *filename);
> int parse_stream_name(char *filename, char **stream_name, int *s_type);
> char *convert_to_nt_pathname(char *filename, char *sharepath);
> int get_nlink(struct kstat *st);
> -void ksmbd_conv_path_to_unix(char *path);
> -void ksmbd_strip_last_slash(char *path);
> +char *ksmbd_conv_path_to_unix(char *path);
> void ksmbd_conv_path_to_windows(char *path);
> char *ksmbd_extract_sharename(char *treename);
> char *convert_to_unix_name(struct ksmbd_share_config *share, char *name);
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index c86164dc70bb..46e0275a77a8 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -634,7 +634,7 @@ static char *
> smb2_get_name(struct ksmbd_share_config *share, const char *src,
> const int maxlen, struct nls_table *local_nls)
> {
> - char *name, *unixname;
> + char *name, *norm_name, *unixname;
>
> name = smb_strndup_from_utf16(src, maxlen, 1, local_nls);
> if (IS_ERR(name)) {
> @@ -643,11 +643,15 @@ smb2_get_name(struct ksmbd_share_config *share, const
> char *src,
> }
>
> /* change it to absolute unix name */
> - ksmbd_conv_path_to_unix(name);
> - ksmbd_strip_last_slash(name);
> -
> - unixname = convert_to_unix_name(share, name);
> + norm_name = ksmbd_conv_path_to_unix(name);
> + if (IS_ERR(norm_name)) {
> + kfree(name);
> + return norm_name;
> + }
> kfree(name);
> +
> + unixname = convert_to_unix_name(share, norm_name);
> + kfree(norm_name);
> if (!unixname) {
> pr_err("can not convert absolute name\n");
> return ERR_PTR(-ENOMEM);
> --
> 2.17.1
>
>
