From 4477e23de60d055fde4a89ebb35691a2f35854fb Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 2 Jan 2024 14:34:26 +0100
Subject: [PATCH] smbd: use safe_symlink_target_path() in
 symlink_target_below_conn()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit 1965fc77b3852a0593e13897af08f5304a1ce3a2)
---
 selftest/skip.opath-required |  2 --
 source3/smbd/open.c          | 73 +++++++++-----------------------------------
 2 files changed, 14 insertions(+), 61 deletions(-)

diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required
index 67764a0b027..9c6ba481cdf 100644
--- a/selftest/skip.opath-required
+++ b/selftest/skip.opath-required
@@ -16,7 +16,5 @@
 ^samba.tests.smb1posix
 
 # These don't work without /proc/fd support
-^samba3.blackbox.test_symlink_traversal.*\(fileserver\)
 ^samba3.blackbox.shadow_copy_torture.*\(fileserver\)
 ^samba3.blackbox.virus_scanner.*\(fileserver:local\)
-^samba3.blackbox.shadow_copy2.*\(fileserver.*\)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index a7759c349e4..27c96c5de2e 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -571,7 +571,6 @@ out:
 static NTSTATUS symlink_target_below_conn(
 	TALLOC_CTX *mem_ctx,
 	const char *connection_path,
-	size_t connection_path_len,
 	struct files_struct *fsp,
 	struct files_struct *dirfsp,
 	struct smb_filename *symlink_name,
@@ -579,9 +578,7 @@ static NTSTATUS symlink_target_below_conn(
 {
 	char *target = NULL;
 	char *absolute = NULL;
-	const char *relative = NULL;
 	NTSTATUS status;
-	bool ok;
 
 	if (fsp_get_pathref_fd(fsp) != -1) {
 		/*
@@ -594,69 +591,28 @@ static NTSTATUS symlink_target_below_conn(
 			talloc_tos(), dirfsp, symlink_name, &target);
 	}
 
+	status = safe_symlink_target_path(talloc_tos(),
+					  connection_path,
+					  dirfsp->fsp_name->base_name,
+					  target,
+					  0,
+					  &absolute);
 	if (!NT_STATUS_IS_OK(status)) {
-		DBG_DEBUG("readlink_talloc failed: %s\n", nt_errstr(status));
+		DBG_DEBUG("safe_symlink_target_path() failed: %s\n",
+			  nt_errstr(status));
 		return status;
 	}
 
-	if (target[0] != '/') {
-		char *tmp = talloc_asprintf(
-			talloc_tos(),
-			"%s/%s/%s",
-			connection_path,
-			dirfsp->fsp_name->base_name,
-			target);
-
-		TALLOC_FREE(target);
-
-		if (tmp == NULL) {
-			return NT_STATUS_NO_MEMORY;
-		}
-		target = tmp;
-	}
-
-	DBG_DEBUG("redirecting to %s\n", target);
-
-	absolute = canonicalize_absolute_path(talloc_tos(), target);
-	TALLOC_FREE(target);
-
-	if (absolute == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	/*
-	 * We're doing the "below connection_path" here because it's
-	 * cheap. It might be that we get a symlink out of the share,
-	 * pointing to yet another symlink getting us back into the
-	 * share. If we need that, we would have to remove the check
-	 * here.
-	 */
-	ok = subdir_of(
-		connection_path,
-		connection_path_len,
-		absolute,
-		&relative);
-	if (!ok) {
-		DBG_NOTICE("Bad access attempt: %s is a symlink "
-			   "outside the share path\n"
-			   "conn_rootdir =%s\n"
-			   "resolved_name=%s\n",
-			   symlink_name->base_name,
-			   connection_path,
-			   absolute);
-		TALLOC_FREE(absolute);
-		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
-	}
-
-	if (relative[0] == '\0') {
+	if (absolute[0] == '\0') {
 		/*
 		 * special case symlink to share root: "." is our
 		 * share root filename
 		 */
-		absolute[0] = '.';
-		absolute[1] = '\0';
-	} else {
-		memmove(absolute, relative, strlen(relative)+1);
+		TALLOC_FREE(absolute);
+		absolute = talloc_strdup(talloc_tos(), ".");
+		if (absolute == NULL) {
+			return NT_STATUS_NO_MEMORY;
+		}
 	}
 
 	*_target = absolute;
@@ -834,7 +790,6 @@ again:
 	status = symlink_target_below_conn(
 		talloc_tos(),
 		connpath,
-		connpath_len,
 		fsp,
 		discard_const_p(files_struct, dirfsp),
 		smb_fname_rel,
-- 
2.11.4.GIT