From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 11:37:17 +0000 (+0200)
Subject: CVE-2022-3592 smbd: Slightly simplify filename_convert_dirfsp()
X-Git-Tag: talloc-2.4.0~650
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d385058ce7c9914ea58613f65414e45f2f777481;p=thirdparty%2Fsamba.git

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>

Autobuild-User(master): Jule Anger <janger@samba.org>
Autobuild-Date(master): Tue Oct 25 11:27:02 UTC 2022 on sn-devel-184
---

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;