Namjae's updated version merged into smb3-kernel cifsd-for-next branch.
Also starting additional tests on it
On Fri, Sep 17, 2021 at 8:18 AM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote:
>
> 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
> >
> >
--
Thanks,
Steve
