From 36fff1ba12176a26678a9efe7bfd8734e694e853 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/4] 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 179c3e11a76b..9b0c902c0d4d 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);
 			status = NT_STATUS_OBJECT_NAME_INVALID;
-- 
2.37.3


From 83c11d8c6046e0586b24252e2dbe1f4bd932457c 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/4] 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 971d53442161..5caee329baf3 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"
 	ln -s "dir_inside_share_noperms" "symlink_dir_inside_share_noperms"
 	mkdir "dir_inside_share_noperms/noperm_subdir_exists"
 	touch "dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists"
+
+	# 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.37.3


From dcf76926e107da16767b30ee743e4736e7e7ea20 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/4] CVE-2022-3592 lib: add subdir_of() to
 source3/lib/util_path.c

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207
Signed-off-by: Volker Lendecke <vl@samba.org>
---
 source3/lib/util_path.c | 51 +++++++++++++++++++++++++++++++++++++++++
 source3/lib/util_path.h |  4 ++++
 2 files changed, 55 insertions(+)

diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
index 3591589cb8e1..b8dfec40b03e 100644
--- a/source3/lib/util_path.c
+++ b/source3/lib/util_path.c
@@ -23,6 +23,7 @@
 
 #include "replace.h"
 #include <talloc.h>
+#include "lib/util/debug.h"
 #include "lib/util/samba_util.h"
 #include "lib/util_path.h"
 
@@ -204,3 +205,53 @@ char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *pathname_in)
 	*p++ = '\0';
 	return pathname;
 }
+
+/*
+ * 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 3e7d04de5507..0ea508bf5bbc 100644
--- a/source3/lib/util_path.h
+++ b/source3/lib/util_path.h
@@ -31,5 +31,9 @@ char *lock_path(TALLOC_CTX *mem_ctx, const char *name);
 char *state_path(TALLOC_CTX *mem_ctx, const char *name);
 char *cache_path(TALLOC_CTX *mem_ctx, const char *name);
 char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path);
+bool subdir_of(const char *parent,
+	       size_t parent_len,
+	       const char *subdir,
+	       const char **_relative);
 
 #endif
-- 
2.37.3


From a7755f5573a75673c2f652e60c89464443b9619b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 13:37:17 +0200
Subject: [PATCH 4/4] 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 5caee329baf3..1fb9c67420d9 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 0be8e320ffa0..e7873eb124f0 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1440,6 +1440,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;
@@ -1512,17 +1513,17 @@ NTSTATUS filename_convert_dirfsp(
 
 	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.37.3