Commit Message

Steve French Oct. 3, 2019, 10:38 p.m. UTC

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: <[e-mail]>
Signed-off-by: Paulo Alcantara (SUSE) <[e-mail]>
Signed-off-by: Aurelien Aptel <[e-mail]>
Signed-off-by: Steve French <[e-mail]>
---
 fs/cifs/readdir.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

  cifs_unix_basic_to_fattr(&fattr,

Patch

From 66eb352a709af18134489f1727c2404dd0e7b033 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <[e-mail]>
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: <[e-mail]>
Signed-off-by: Paulo Alcantara (SUSE) <[e-mail]>
Signed-off-by: Aurelien Aptel <[e-mail]>
Signed-off-by: Steve French <[e-mail]>
---
 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