Skip to content

Commit

Permalink
ksmbd: prevent out of share access
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Hyunchul Lee authored and Steve French committed Sep 17, 2021
1 parent a9b3043 commit f58eae6
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 16 deletions.
76 changes: 67 additions & 9 deletions fs/ksmbd/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions fs/ksmbd/misc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 9 additions & 5 deletions fs/ksmbd/smb2pdu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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);
Expand Down

0 comments on commit f58eae6

Please sign in to comment.