Haven't heard anything recently on this (although something similar was apparently discussed last month on various other mailing lists) so posting Aurelien's patch for external review/comments before deciding whether to put in cifs's for-next branch. One question is whether the check should (only) be done at the higher (VFS) layer, but if ok to check at (potentially both the layer above, the VFS and ) the individual fs level, I would prefer to get this patch or something similar in pretty soon into cifs.ko. Although cifs.ko is probably less at risk due to signing and encryption - the idea seems fine to protect against / in path components. [PATCH] cifs: do not accept filenames containing dir separators Check for / in all connection types and additionally check for \ in non-posix paths connections. By returning early we do not add this directory entry via dir_emits(), essentially skipping it. Since the code relies on ctx->pos being incremented regardless of errors, we return 0. This fix addresses CVE-2019-10220. Link: https://bugzilla.samba.org/show_bug.cgi?id=14072 CC: <stable@xxxxxxxxxxxxxxx> Signed-off-by: Paulo Alcantara (SUSE) <paulo@xxxxxxxx> Signed-off-by: Aurelien Aptel <aaptel@xxxxxxxx> Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> --- fs/cifs/readdir.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 3925a7bfc74d..30d69a9d3e94 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -744,6 +744,20 @@ static int cifs_filldir(char *find_entry, struct file *file, name.len = de.namelen; } + /* + * Regardless of connection type, / is always forbidden + * IFF we use normal windows paths then \ is forbidden + */ + + if (strnchr(name.name, name.len, '/') + || (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) + && strnchr(name.name, name.len, '\\'))) { + cifs_dbg(VFS, "server returned name containing dir separator"); + /* skip this entry for next readdir() interaction */ + file_info->srch_inf.entries_in_buffer--; + return 0; + } + switch (file_info->srch_inf.info_level) { case SMB_FIND_FILE_UNIX: cifs_unix_basic_to_fattr(&fattr, -- Thanks, Steve
From 66eb352a709af18134489f1727c2404dd0e7b033 Mon Sep 17 00:00:00 2001 From: Aurelien Aptel <aaptel@xxxxxxxx> Date: Thu, 8 Aug 2019 18:42:17 +0200 Subject: [PATCH] cifs: do not accept filenames containing dir separators Check for / in all connection types and additionally check for \ in non-posix paths connections. By returning early we do not add this directory entry via dir_emits(), essentially skipping it. Since the code relies on ctx->pos being incremented regardless of errors, we return 0. This fix addresses CVE-2019-10220. Link: https://bugzilla.samba.org/show_bug.cgi?id=14072 CC: <stable@xxxxxxxxxxxxxxx> Signed-off-by: Paulo Alcantara (SUSE) <paulo@xxxxxxxx> Signed-off-by: Aurelien Aptel <aaptel@xxxxxxxx> Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> --- fs/cifs/readdir.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 3925a7bfc74d..30d69a9d3e94 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -744,6 +744,20 @@ static int cifs_filldir(char *find_entry, struct file *file, name.len = de.namelen; } + /* + * Regardless of connection type, / is always forbidden + * IFF we use normal windows paths then \ is forbidden + */ + + if (strnchr(name.name, name.len, '/') + || (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) + && strnchr(name.name, name.len, '\\'))) { + cifs_dbg(VFS, "server returned name containing dir separator"); + /* skip this entry for next readdir() interaction */ + file_info->srch_inf.entries_in_buffer--; + return 0; + } + switch (file_info->srch_inf.info_level) { case SMB_FIND_FILE_UNIX: cifs_unix_basic_to_fattr(&fattr, -- 2.20.1
