ksmbd: prevent out of share access

Because of .., files outside the share directory
could be accessed. To prevent this, normalize
the given path and remove all . and ..
components.

In addition to the usual large set of regression tests (smbtorture
and xfstests), ran various tests on this to specifically check
path name validation including libsmb2 tests to verify path
normalization:

 ./examples/smb2-ls-async smb://172.30.1.15/homes2/../
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../../
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/..bar/
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar../
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar..
 ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar../../../../

Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/ksmbd/misc.c b/fs/ksmbd/misc.c
index 0b307ca28a19d99ef8371bb71cd200495f3a4c96..3eac3c01749fa0e74b4fecb982b37ad5331f0396 100644 (file)
--- a/fs/ksmbd/misc.c
+++ b/fs/ksmbd/misc.c
@@ -191,19 +191,77 @@ 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 == pre_dotdot_cnt + 1)
+                                               break;
+                               }
+
+                               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 af8717d4d85b32d725a6b7496be397afe32d2a7c..b7b10139ada213fb23a751cf225661968fa19ffd 100644 (file)
--- 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 c86164dc70bb776732f6781b45bd50b691898787..46e0275a77a8cebfede69f163ae6d269774705bb 100644 (file)
--- 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);