Skip to content

Commit f58eae6

Browse files
hcleesmfrench
authored andcommitted
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 <[email protected]> Signed-off-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent a9b3043 commit f58eae6

File tree

3 files changed

+77
-16
lines changed

3 files changed

+77
-16
lines changed

fs/ksmbd/misc.c

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -191,19 +191,77 @@ int get_nlink(struct kstat *st)
191191
return nlink;
192192
}
193193

194-
void ksmbd_conv_path_to_unix(char *path)
194+
char *ksmbd_conv_path_to_unix(char *path)
195195
{
196+
size_t path_len, remain_path_len, out_path_len;
197+
char *out_path, *out_next;
198+
int i, pre_dotdot_cnt = 0, slash_cnt = 0;
199+
bool is_last;
200+
196201
strreplace(path, '\\', '/');
197-
}
202+
path_len = strlen(path);
203+
remain_path_len = path_len;
204+
if (path_len == 0)
205+
return ERR_PTR(-EINVAL);
198206

199-
void ksmbd_strip_last_slash(char *path)
200-
{
201-
int len = strlen(path);
207+
out_path = kzalloc(path_len + 2, GFP_KERNEL);
208+
if (!out_path)
209+
return ERR_PTR(-ENOMEM);
210+
out_path_len = 0;
211+
out_next = out_path;
212+
213+
do {
214+
char *name = path + path_len - remain_path_len;
215+
char *next = strchrnul(name, '/');
216+
size_t name_len = next - name;
217+
218+
is_last = !next[0];
219+
if (name_len == 2 && name[0] == '.' && name[1] == '.') {
220+
pre_dotdot_cnt++;
221+
/* handle the case that path ends with "/.." */
222+
if (is_last)
223+
goto follow_dotdot;
224+
} else {
225+
if (pre_dotdot_cnt) {
226+
follow_dotdot:
227+
slash_cnt = 0;
228+
for (i = out_path_len - 1; i >= 0; i--) {
229+
if (out_path[i] == '/' &&
230+
++slash_cnt == pre_dotdot_cnt + 1)
231+
break;
232+
}
233+
234+
if (i < 0 &&
235+
slash_cnt != pre_dotdot_cnt) {
236+
kfree(out_path);
237+
return ERR_PTR(-EINVAL);
238+
}
239+
240+
out_next = &out_path[i+1];
241+
*out_next = '\0';
242+
out_path_len = i + 1;
202243

203-
while (len && path[len - 1] == '/') {
204-
path[len - 1] = '\0';
205-
len--;
206-
}
244+
}
245+
246+
if (name_len != 0 &&
247+
!(name_len == 1 && name[0] == '.') &&
248+
!(name_len == 2 && name[0] == '.' && name[1] == '.')) {
249+
next[0] = '\0';
250+
sprintf(out_next, "%s/", name);
251+
out_next += name_len + 1;
252+
out_path_len += name_len + 1;
253+
next[0] = '/';
254+
}
255+
pre_dotdot_cnt = 0;
256+
}
257+
258+
remain_path_len -= name_len + 1;
259+
} while (!is_last);
260+
261+
if (out_path_len > 0)
262+
out_path[out_path_len-1] = '\0';
263+
path[path_len] = '\0';
264+
return out_path;
207265
}
208266

209267
void ksmbd_conv_path_to_windows(char *path)

fs/ksmbd/misc.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ int ksmbd_validate_filename(char *filename);
1616
int parse_stream_name(char *filename, char **stream_name, int *s_type);
1717
char *convert_to_nt_pathname(char *filename, char *sharepath);
1818
int get_nlink(struct kstat *st);
19-
void ksmbd_conv_path_to_unix(char *path);
20-
void ksmbd_strip_last_slash(char *path);
19+
char *ksmbd_conv_path_to_unix(char *path);
2120
void ksmbd_conv_path_to_windows(char *path);
2221
char *ksmbd_extract_sharename(char *treename);
2322
char *convert_to_unix_name(struct ksmbd_share_config *share, char *name);

fs/ksmbd/smb2pdu.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ static char *
634634
smb2_get_name(struct ksmbd_share_config *share, const char *src,
635635
const int maxlen, struct nls_table *local_nls)
636636
{
637-
char *name, *unixname;
637+
char *name, *norm_name, *unixname;
638638

639639
name = smb_strndup_from_utf16(src, maxlen, 1, local_nls);
640640
if (IS_ERR(name)) {
@@ -643,11 +643,15 @@ smb2_get_name(struct ksmbd_share_config *share, const char *src,
643643
}
644644

645645
/* change it to absolute unix name */
646-
ksmbd_conv_path_to_unix(name);
647-
ksmbd_strip_last_slash(name);
648-
649-
unixname = convert_to_unix_name(share, name);
646+
norm_name = ksmbd_conv_path_to_unix(name);
647+
if (IS_ERR(norm_name)) {
648+
kfree(name);
649+
return norm_name;
650+
}
650651
kfree(name);
652+
653+
unixname = convert_to_unix_name(share, norm_name);
654+
kfree(norm_name);
651655
if (!unixname) {
652656
pr_err("can not convert absolute name\n");
653657
return ERR_PTR(-ENOMEM);

0 commit comments

Comments
 (0)