Skip to content

Commit

Permalink
ksmbd: use LOOKUP_BENEATH to prevent the out of share access
Browse files Browse the repository at this point in the history 
instead of removing '..' in a given path, call
kern_path with LOOKUP_BENEATH flag to prevent
the out of share access.

ran various test on this:
smb2-cat-async smb://127.0.0.1/homes/../out_of_share
smb2-cat-async smb://127.0.0.1/homes/foo/../../out_of_share
smbclient //127.0.0.1/homes -c "mkdir ../foo2"
smbclient //127.0.0.1/homes -c "rename bar ../bar"

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Boehme <slow@samba.org>
Tested-by: Steve French <smfrench@gmail.com>
Tested-by: Namjae Jeon <linkinjeon@kernel.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
  • Loading branch information
Hyunchul Lee authored and Steve French committed Sep 25, 2021
1 parent 4ea4779 commit 265fd19
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 206 deletions.
100 changes: 19 additions & 81 deletions fs/ksmbd/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,25 +158,21 @@ int parse_stream_name(char *filename, char **stream_name, int *s_type)
* Return : windows path string or error
*/

char *convert_to_nt_pathname(char *filename, char *sharepath)
char *convert_to_nt_pathname(char *filename)
{
char *ab_pathname;
int len, name_len;

name_len = strlen(filename);
ab_pathname = kmalloc(name_len, GFP_KERNEL);
if (!ab_pathname)
return NULL;

ab_pathname[0] = '\\';
ab_pathname[1] = '\0';
if (strlen(filename) == 0) {
ab_pathname = kmalloc(2, GFP_KERNEL);
ab_pathname[0] = '\\';
ab_pathname[1] = '\0';
} else {
ab_pathname = kstrdup(filename, GFP_KERNEL);
if (!ab_pathname)
return NULL;

len = strlen(sharepath);
if (!strncmp(filename, sharepath, len) && name_len != len) {
strscpy(ab_pathname, &filename[len], name_len);
ksmbd_conv_path_to_windows(ab_pathname);
}

return ab_pathname;
}

Expand All @@ -191,77 +187,19 @@ int get_nlink(struct kstat *st)
return nlink;
}

char *ksmbd_conv_path_to_unix(char *path)
void 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);

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;

}

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);
void ksmbd_strip_last_slash(char *path)
{
int len = strlen(path);

if (out_path_len > 0)
out_path[out_path_len-1] = '\0';
path[path_len] = '\0';
return out_path;
while (len && path[len - 1] == '/') {
path[len - 1] = '\0';
len--;
}
}

void ksmbd_conv_path_to_windows(char *path)
Expand Down Expand Up @@ -298,7 +236,7 @@ char *ksmbd_extract_sharename(char *treename)
*
* Return: converted name on success, otherwise NULL
*/
char *convert_to_unix_name(struct ksmbd_share_config *share, char *name)
char *convert_to_unix_name(struct ksmbd_share_config *share, const char *name)
{
int no_slash = 0, name_len, path_len;
char *new_name;
Expand Down
7 changes: 4 additions & 3 deletions fs/ksmbd/misc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ struct ksmbd_file;
int match_pattern(const char *str, size_t len, const char *pattern);
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);
char *convert_to_nt_pathname(char *filename);
int get_nlink(struct kstat *st);
char *ksmbd_conv_path_to_unix(char *path);
void ksmbd_conv_path_to_unix(char *path);
void ksmbd_strip_last_slash(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);
char *convert_to_unix_name(struct ksmbd_share_config *share, const char *name);

#define KSMBD_DIR_INFO_ALIGNMENT 8
struct ksmbd_dir_info;
Expand Down
74 changes: 28 additions & 46 deletions fs/ksmbd/smb2pdu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,31 +634,17 @@ static char *
smb2_get_name(struct ksmbd_share_config *share, const char *src,
const int maxlen, struct nls_table *local_nls)
{
char *name, *norm_name, *unixname;
char *name;

name = smb_strndup_from_utf16(src, maxlen, 1, local_nls);
if (IS_ERR(name)) {
pr_err("failed to get name %ld\n", PTR_ERR(name));
return name;
}

/* change it to absolute unix 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);
}

ksmbd_debug(SMB, "absolute name = %s\n", unixname);
return unixname;
ksmbd_conv_path_to_unix(name);
ksmbd_strip_last_slash(name);
return name;
}

int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg)
Expand Down Expand Up @@ -2352,7 +2338,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *path, char *name,
return rc;
}

rc = ksmbd_vfs_kern_path(name, 0, path, 0);
rc = ksmbd_vfs_kern_path(work, name, 0, path, 0);
if (rc) {
pr_err("cannot get linux path (%s), err = %d\n",
name, rc);
Expand Down Expand Up @@ -2427,7 +2413,7 @@ int smb2_open(struct ksmbd_work *work)
struct oplock_info *opinfo;
__le32 *next_ptr = NULL;
int req_op_level = 0, open_flags = 0, may_flags = 0, file_info = 0;
int rc = 0, len = 0;
int rc = 0;
int contxt_cnt = 0, query_disk_id = 0;
int maximal_access_ctxt = 0, posix_ctxt = 0;
int s_type = 0;
Expand Down Expand Up @@ -2499,17 +2485,11 @@ int smb2_open(struct ksmbd_work *work)
goto err_out1;
}
} else {
len = strlen(share->path);
ksmbd_debug(SMB, "share path len %d\n", len);
name = kmalloc(len + 1, GFP_KERNEL);
name = kstrdup("", GFP_KERNEL);
if (!name) {
rsp->hdr.Status = STATUS_NO_MEMORY;
rc = -ENOMEM;
goto err_out1;
}

memcpy(name, share->path, len);
*(name + len) = '\0';
}

req_op_level = req->RequestedOplockLevel;
Expand Down Expand Up @@ -2632,7 +2612,7 @@ int smb2_open(struct ksmbd_work *work)
goto err_out1;
}

rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1);
rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1);
if (!rc) {
if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
/*
Expand Down Expand Up @@ -2661,11 +2641,8 @@ int smb2_open(struct ksmbd_work *work)
}

if (rc) {
if (rc == -EACCES) {
ksmbd_debug(SMB,
"User does not have right permission\n");
if (rc != -ENOENT)
goto err_out;
}
ksmbd_debug(SMB, "can not get linux path for %s, rc = %d\n",
name, rc);
rc = 0;
Expand Down Expand Up @@ -3161,7 +3138,7 @@ int smb2_open(struct ksmbd_work *work)
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
else if (rc == -EOPNOTSUPP)
rsp->hdr.Status = STATUS_NOT_SUPPORTED;
else if (rc == -EACCES || rc == -ESTALE)
else if (rc == -EACCES || rc == -ESTALE || rc == -EXDEV)
rsp->hdr.Status = STATUS_ACCESS_DENIED;
else if (rc == -ENOENT)
rsp->hdr.Status = STATUS_OBJECT_NAME_INVALID;
Expand Down Expand Up @@ -4277,8 +4254,7 @@ static int get_file_all_info(struct ksmbd_work *work,
return -EACCES;
}

filename = convert_to_nt_pathname(fp->filename,
work->tcon->share_conf->path);
filename = convert_to_nt_pathname(fp->filename);
if (!filename)
return -ENOMEM;

Expand Down Expand Up @@ -4733,7 +4709,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work,
int rc = 0, len;
int fs_infoclass_size = 0;

rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0);
rc = kern_path(share->path, LOOKUP_NO_SYMLINKS, &path);
if (rc) {
pr_err("cannot create vfs path\n");
return -EIO;
Expand Down Expand Up @@ -5282,7 +5258,7 @@ static int smb2_rename(struct ksmbd_work *work,
goto out;

len = strlen(new_name);
if (new_name[len - 1] != '/') {
if (len > 0 && new_name[len - 1] != '/') {
pr_err("not allow base filename in rename\n");
rc = -ESHARE;
goto out;
Expand Down Expand Up @@ -5310,11 +5286,14 @@ static int smb2_rename(struct ksmbd_work *work,
}

ksmbd_debug(SMB, "new name %s\n", new_name);
rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1);
if (rc)
rc = ksmbd_vfs_kern_path(work, new_name, LOOKUP_NO_SYMLINKS, &path, 1);
if (rc) {
if (rc != -ENOENT)
goto out;
file_present = false;
else
} else {
path_put(&path);
}

if (ksmbd_share_veto_filename(share, new_name)) {
rc = -ENOENT;
Expand Down Expand Up @@ -5384,11 +5363,14 @@ static int smb2_create_link(struct ksmbd_work *work,
}

ksmbd_debug(SMB, "target name is %s\n", target_name);
rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0);
if (rc)
rc = ksmbd_vfs_kern_path(work, link_name, LOOKUP_NO_SYMLINKS, &path, 0);
if (rc) {
if (rc != -ENOENT)
goto out;
file_present = false;
else
} else {
path_put(&path);
}

if (file_info->ReplaceIfExists) {
if (file_present) {
Expand Down Expand Up @@ -5548,7 +5530,7 @@ static int set_file_allocation_info(struct ksmbd_work *work,
* inode size is retained by backup inode size.
*/
size = i_size_read(inode);
rc = ksmbd_vfs_truncate(work, NULL, fp, alloc_blks * 512);
rc = ksmbd_vfs_truncate(work, fp, alloc_blks * 512);
if (rc) {
pr_err("truncate failed! filename : %s, err %d\n",
fp->filename, rc);
Expand Down Expand Up @@ -5585,7 +5567,7 @@ static int set_end_of_file_info(struct ksmbd_work *work, struct ksmbd_file *fp,
if (inode->i_sb->s_magic != MSDOS_SUPER_MAGIC) {
ksmbd_debug(SMB, "filename : %s truncated to newsize %lld\n",
fp->filename, newsize);
rc = ksmbd_vfs_truncate(work, NULL, fp, newsize);
rc = ksmbd_vfs_truncate(work, fp, newsize);
if (rc) {
ksmbd_debug(SMB, "truncate failed! filename : %s err %d\n",
fp->filename, rc);
Expand Down Expand Up @@ -5862,7 +5844,7 @@ int smb2_set_info(struct ksmbd_work *work)
return 0;

err_out:
if (rc == -EACCES || rc == -EPERM)
if (rc == -EACCES || rc == -EPERM || rc == -EXDEV)
rsp->hdr.Status = STATUS_ACCESS_DENIED;
else if (rc == -EINVAL)
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
Expand Down
Loading

0 comments on commit 265fd19

Please sign in to comment.