From 67f01aa30fad181d4b43b329b1cab15c9430a8af Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Mon, 17 Oct 2022 18:06:02 +0200
Subject: [PATCH 1/5] CVE-2022-3592 smbd: No empty path components in
 openat_pathref_dirfsp_nosymlink()

Upper layers must have filtered this, everything else is a bug

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/smbd/files.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 87c1f0f6301..64297f18773 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -817,6 +817,12 @@ NTSTATUS openat_pathref_dirfsp_nosymlink(
 
 		next = strv_next(path, rel_fname.base_name);
 
+		/*
+		 * Path sanitizing further up has cleaned or rejected
+		 * empty path components. Assert this here.
+		 */
+		SMB_ASSERT(rel_fname.base_name[0] != '\0');
+
 		if (ISDOT(rel_fname.base_name) ||
 		    ISDOTDOT(rel_fname.base_name)) {
 			DBG_DEBUG("%s contains a dot\n", path_in);
-- 
2.30.2


From 9fa26b07c57cbc7dc8f7b7891d0a572cb26cee55 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 14:09:55 +0200
Subject: [PATCH 2/5] CVE-2022-3592 torture3: Show that our symlink traversal
 checks are insecure

This test shows that we don't properly check whether symlink targets
are inside the exported share. Linking to <share-root>a/etc makes us
loop back into filename_convert_dirfsp_nosymlink() with /etc as a
directory name.

On Linux systems with openat2(RESOLVE_NO_SYMLINKS) we pass "/etc"
directly into that call after some checks for "."/".." as invalid file
name components. "/etc" is okay for openat2(), but this test must also
succeed on systems without RESOLVE_NO_SYMLINKS (sn-devel-184 for
example). On systems without RESOLVE_NO_SYMLINKS split up the path
"/etc" into path components, in this case "" and "etc". So we pass ""
down to openat(), which correctly fails with ENOENT.

Summary: Only with RESOLVE_NO_SYMLINKS we're hit by bug 15207, and
this test shows by expecting CONNECTION_DISCONNECTED that we violate
the internal assumption of empty path components with an unexpected
symlink target, making it testable on systems with and without
RESOLVE_NO_SYMLINKS.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/script/tests/test_symlink_traversal_smb2.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh
index efd4353c533..08929f0962f 100755
--- a/source3/script/tests/test_symlink_traversal_smb2.sh
+++ b/source3/script/tests/test_symlink_traversal_smb2.sh
@@ -144,6 +144,9 @@ chmod 0 "$dir_outside_share_noperms"
 	mkdir "dir_inside_share_noperms/noperm_subdir_exists"
 	touch "dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists"
 	chmod 0 "dir_inside_share_noperms"
+
+	# Symlink pointing out of the share
+	ln -s "$share_test_dir"a"/etc" x
 )
 
 #
@@ -345,6 +348,7 @@ test_symlink_traversal_SMB2()
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1
+	smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_CONNECTION_DISCONNECTED" || return 1
 
 	#
 	# Test paths within share with no permissions.
-- 
2.30.2


From f86c444b7ab56e174e3e25ace81de0f497f476f0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 13:26:48 +0200
Subject: [PATCH 3/5] CVE-2022-3592 lib: lib/util/fault.h requires
 _SAMBA_DEBUG_H for SMB_ASSERT()

fault.h has:

which leads to SMB_ASSERT not being defined when you include
samba_util.h (and thus fault.h) before debug.h.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/lib/util_path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
index 5b7c01b05e3..33827fcf894 100644
--- a/source3/lib/util_path.c
+++ b/source3/lib/util_path.c
@@ -23,8 +23,8 @@
 
 #include "replace.h"
 #include <talloc.h>
-#include "lib/util/samba_util.h"
 #include "lib/util/debug.h"
+#include "lib/util/samba_util.h"
 #include "lib/util_path.h"
 
 struct loadparm_substitution;
-- 
2.30.2


From 964a76dc00c02aed21ddcf23cf09a986accf92c4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 13:29:14 +0200
Subject: [PATCH 4/5] CVE-2022-3592 lib: Move subdir_of() to
 source3/lib/util_path.c

Make it available for other components

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/lib/util_path.c | 50 +++++++++++++++++++++++++++++++++++++++
 source3/lib/util_path.h |  4 ++++
 source3/smbd/open.c     | 52 -----------------------------------------
 3 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
index 33827fcf894..5a94b391dd6 100644
--- a/source3/lib/util_path.c
+++ b/source3/lib/util_path.c
@@ -304,3 +304,53 @@ bool extract_snapshot_token(char *fname, NTTIME *twrp)
 
 	return true;
 }
+
+/*
+ * Take two absolute paths, figure out if "subdir" is a proper
+ * subdirectory of "parent". Return the component relative to the
+ * "parent" without the potential "/". Take care of "parent"
+ * possibly ending in "/".
+ */
+bool subdir_of(const char *parent,
+	       size_t parent_len,
+	       const char *subdir,
+	       const char **_relative)
+{
+	const char *relative = NULL;
+	bool matched;
+
+	SMB_ASSERT(parent[0] == '/');
+	SMB_ASSERT(subdir[0] == '/');
+
+	if (parent_len == 1) {
+		/*
+		 * Everything is below "/"
+		 */
+		*_relative = subdir+1;
+		return true;
+	}
+
+	if (parent[parent_len-1] == '/') {
+		parent_len -= 1;
+	}
+
+	matched = (strncmp(subdir, parent, parent_len) == 0);
+	if (!matched) {
+		return false;
+	}
+
+	relative = &subdir[parent_len];
+
+	if (relative[0] == '\0') {
+		*_relative = relative; /* nothing left */
+		return true;
+	}
+
+	if (relative[0] == '/') {
+		/* End of parent must match a '/' in subdir. */
+		*_relative = relative+1;
+		return true;
+	}
+
+	return false;
+}
diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h
index 33699da28c2..5b0a1f13e38 100644
--- a/source3/lib/util_path.h
+++ b/source3/lib/util_path.h
@@ -49,5 +49,9 @@ bool clistr_is_previous_version_path(const char *path,
 				     const char **startp,
 				     const char **endp,
 				     NTTIME *ptwrp);
+bool subdir_of(const char *parent,
+	       size_t parent_len,
+	       const char *subdir,
+	       const char **_relative);
 
 #endif
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 9d85b0d8ef8..d4a679a4cb0 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -475,58 +475,6 @@ static NTSTATUS check_base_file_access(struct files_struct *fsp,
 					access_mask);
 }
 
-/*
- * Take two absolute paths, figure out if "subdir" is a proper
- * subdirectory of "parent". Return the component relative to the
- * "parent" without the potential "/". Take care of "parent"
- * possibly ending in "/".
- */
-static bool subdir_of(
-	const char *parent,
-	size_t parent_len,
-	const char *subdir,
-	const char **_relative)
-
-{
-	const char *relative = NULL;
-	bool matched;
-
-	SMB_ASSERT(parent[0] == '/');
-	SMB_ASSERT(subdir[0] == '/');
-
-	if (parent_len == 1) {
-		/*
-		 * Everything is below "/"
-		 */
-		*_relative = subdir+1;
-		return true;
-	}
-
-	if (parent[parent_len-1] == '/') {
-		parent_len -= 1;
-	}
-
-	matched = (strncmp(subdir, parent, parent_len) == 0);
-	if (!matched) {
-		return false;
-	}
-
-	relative = &subdir[parent_len];
-
-	if (relative[0] == '\0') {
-		*_relative = relative; /* nothing left */
-		return true;
-	}
-
-	if (relative[0] == '/') {
-		/* End of parent must match a '/' in subdir. */
-		*_relative = relative+1;
-		return true;
-	}
-
-	return false;
-}
-
 static NTSTATUS chdir_below_conn(
 	TALLOC_CTX *mem_ctx,
 	connection_struct *conn,
-- 
2.30.2


From 865b65de466d2531f8899274f89f597a4574817e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 13:37:17 +0200
Subject: [PATCH 5/5] CVE-2022-3592 smbd: Slightly simplify
 filename_convert_dirfsp()

subdir_of() calculates the share-relative rest for us, don't do the
strlen(connectpath) calculation twice. subdir_of() also checks that
the target properly ends on a directory. With just strncmp a symlink
to x->/aa/etc would qualify as in share /a, so a "get x/passwd" leads to a
pretty unfortunate result. This is the proper fix for bug 15207, so we
need to change the expected error code to OBJECT_PATH_NOT_FOUND

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/script/tests/test_symlink_traversal_smb2.sh |  2 +-
 source3/smbd/filename.c                             | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh
index 08929f0962f..38719cc8eae 100755
--- a/source3/script/tests/test_symlink_traversal_smb2.sh
+++ b/source3/script/tests/test_symlink_traversal_smb2.sh
@@ -348,7 +348,7 @@ test_symlink_traversal_SMB2()
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1
 	smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1
-	smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_CONNECTION_DISCONNECTED" || return 1
+	smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1
 
 	#
 	# Test paths within share with no permissions.
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 79bb7dffe99..5bfa4b2f1df 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1369,6 +1369,7 @@ NTSTATUS filename_convert_dirfsp(
 	struct smb_filename **_smb_fname)
 {
 	char *substitute = NULL;
+	const char *relative = NULL;
 	size_t unparsed = 0;
 	NTSTATUS status;
 	char *target = NULL;
@@ -1441,17 +1442,17 @@ next:
 
 	DBG_DEBUG("abs_target_canon=%s\n", abs_target_canon);
 
-	in_share = strncmp(
-		abs_target_canon,
+	in_share = subdir_of(
 		conn->connectpath,
-		strlen(conn->connectpath)) == 0;
+		strlen(conn->connectpath),
+		abs_target_canon,
+		&relative);
 	if (!in_share) {
 		DBG_DEBUG("wide link to %s\n", abs_target_canon);
 		return NT_STATUS_OBJECT_PATH_NOT_FOUND;
 	}
 
-	name_in = talloc_strdup(
-		mem_ctx, abs_target_canon + strlen(conn->connectpath) + 1);
+	name_in = talloc_strdup(mem_ctx, relative);
 
 	symlink_redirects += 1;
 
-- 
2.30.2