From 1db6b4c854f3171bef570553ebc98ccefe7467e6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 15 Oct 2022 13:29:14 +0200
Subject: [PATCH 01/12] CVE-2023-34968: 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>
(backported from commit d905dbddf8d2655e6c91752b750cbe9c15837ee5)
[slow@samba.org: subdir_of() didn't exist yet in 4.16 so this just adds it]
---
 source3/lib/util_path.c | 52 +++++++++++++++++++++++++++++++++++++++++
 source3/lib/util_path.h |  4 ++++
 2 files changed, 56 insertions(+)

diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
index c34b734384cf..e6bed7245517 100644
--- a/source3/lib/util_path.c
+++ b/source3/lib/util_path.c
@@ -23,6 +23,8 @@
 
 #include "replace.h"
 #include <talloc.h>
+#include "lib/util/debug.h"
+#include "lib/util/fault.h"
 #include "lib/util/samba_util.h"
 #include "lib/util_path.h"
 
@@ -210,3 +212,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.40.0


From 585222748e4120f1079bff3c3eab2516883e90c9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 6 Jun 2023 15:17:26 +0200
Subject: [PATCH 02/12] CVE-2023-34968: mdssvc: cache and reuse stat info in
 struct sl_inode_path_map

Prepare for the "path" being a fake path and not the real server-side
path where we won't be able to vfs_stat_fsp() this fake path. Luckily we already
got stat info for the object in mds_add_result() so we can just pass stat info
from there.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_server/mdssvc/mdssvc.c | 32 +++++++-----------------------
 source3/rpc_server/mdssvc/mdssvc.h |  1 +
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
index 02c422116947..3af0a71a28ea 100644
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -446,7 +446,10 @@ static int ino_path_map_destr_cb(struct sl_inode_path_map *entry)
  * entries by calling talloc_free() on the query slq handles.
  **/
 
-static bool inode_map_add(struct sl_query *slq, uint64_t ino, const char *path)
+static bool inode_map_add(struct sl_query *slq,
+			  uint64_t ino,
+			  const char *path,
+			  struct stat_ex *st)
 {
 	NTSTATUS status;
 	struct sl_inode_path_map *entry;
@@ -493,6 +496,7 @@ static bool inode_map_add(struct sl_query *slq, uint64_t ino, const char *path)
 
 	entry->ino = ino;
 	entry->mds_ctx = slq->mds_ctx;
+	entry->st = *st;
 	entry->path = talloc_strdup(entry, path);
 	if (entry->path == NULL) {
 		DEBUG(1, ("talloc failed\n"));
@@ -630,7 +634,7 @@ bool mds_add_result(struct sl_query *slq, const char *path)
 		return false;
 	}
 
-	ok = inode_map_add(slq, ino64, path);
+	ok = inode_map_add(slq, ino64, path, &sb);
 	if (!ok) {
 		DEBUG(1, ("inode_map_add error\n"));
 		slq->state = SLQ_STATE_ERROR;
@@ -1353,29 +1357,7 @@ static bool slrpc_fetch_attributes(struct mds_ctx *mds_ctx,
 		elem = talloc_get_type_abort(p, struct sl_inode_path_map);
 		path = elem->path;
 
-		status = synthetic_pathref(talloc_tos(),
-					   mds_ctx->conn->cwd_fsp,
-					   path,
-					   NULL,
-					   NULL,
-					   0,
-					   0,
-					   &smb_fname);
-		if (!NT_STATUS_IS_OK(status)) {
-			/* This is not an error, the user may lack permissions */
-			DBG_DEBUG("synthetic_pathref [%s]: %s\n",
-				  smb_fname_str_dbg(smb_fname),
-				  nt_errstr(status));
-			return true;
-		}
-
-		status = vfs_stat_fsp(smb_fname->fsp);
-		if (!NT_STATUS_IS_OK(status)) {
-			TALLOC_FREE(smb_fname);
-			return true;
-		}
-
-		sp = &smb_fname->fsp->fsp_name->st;
+		sp = &elem->st;
 	}
 
 	ok = add_filemeta(mds_ctx, reqinfo, fm_array, path, sp);
diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h
index 205417c4be1c..ff36b329f2b0 100644
--- a/source3/rpc_server/mdssvc/mdssvc.h
+++ b/source3/rpc_server/mdssvc/mdssvc.h
@@ -105,6 +105,7 @@ struct sl_inode_path_map {
 	struct mds_ctx    *mds_ctx;
 	uint64_t           ino;
 	char              *path;
+	struct stat_ex     st;
 };
 
 /* Per process state */
-- 
2.40.0


From 98cde0b26d927c5f0800f51d9a6750fec3992f43 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 17 Jun 2023 13:39:55 +0200
Subject: [PATCH 03/12] CVE-2023-34968: mdssvc: add missing
 "kMDSStoreMetaScopes" dict key in slrpc_fetch_properties()

We were adding the value, but not the key.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_server/mdssvc/mdssvc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
index 3af0a71a28ea..72936a992890 100644
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -743,6 +743,10 @@ static bool slrpc_fetch_properties(struct mds_ctx *mds_ctx,
 	}
 
 	/* kMDSStoreMetaScopes array */
+	result = dalloc_stradd(dict, "kMDSStoreMetaScopes");
+	if (result != 0) {
+		return false;
+	}
 	array = dalloc_zero(dict, sl_array_t);
 	if (array == NULL) {
 		return NULL;
-- 
2.40.0


From f65004a6ed4fc78de5f40d3ab85ee9d1b16f7a3d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 19 Jun 2023 17:14:38 +0200
Subject: [PATCH 04/12] CVE-2023-34968: mdscli: use correct TALLOC memory
 context when allocating spotlight_blob

d is talloc_free()d at the end of the functions and the buffer was later used
after beeing freed in the DCERPC layer when sending the packet.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_client/cli_mdssvc_util.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c
index fe5092c3790a..892a844e71a8 100644
--- a/source3/rpc_client/cli_mdssvc_util.c
+++ b/source3/rpc_client/cli_mdssvc_util.c
@@ -209,7 +209,7 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(d,
+	blob->spotlight_blob = talloc_array(mem_ctx,
 					    uint8_t,
 					    ctx->max_fragment_size);
 	if (blob->spotlight_blob == NULL) {
@@ -293,7 +293,7 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(d,
+	blob->spotlight_blob = talloc_array(mem_ctx,
 					    uint8_t,
 					    ctx->max_fragment_size);
 	if (blob->spotlight_blob == NULL) {
@@ -426,7 +426,7 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(d,
+	blob->spotlight_blob = talloc_array(mem_ctx,
 					    uint8_t,
 					    ctx->max_fragment_size);
 	if (blob->spotlight_blob == NULL) {
@@ -510,7 +510,7 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(d,
+	blob->spotlight_blob = talloc_array(mem_ctx,
 					    uint8_t,
 					    ctx->max_fragment_size);
 	if (blob->spotlight_blob == NULL) {
-- 
2.40.0


From 583ae3d20f99301b0c8905f58c00614b8d39d9c5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 19 Jun 2023 18:28:41 +0200
Subject: [PATCH 05/12] CVE-2023-34968: mdscli: remove response blob allocation

This is handled by the NDR code transparently.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_client/cli_mdssvc.c | 36 ---------------------------------
 1 file changed, 36 deletions(-)

diff --git a/source3/rpc_client/cli_mdssvc.c b/source3/rpc_client/cli_mdssvc.c
index 82d14372fe45..07c19b51dd49 100644
--- a/source3/rpc_client/cli_mdssvc.c
+++ b/source3/rpc_client/cli_mdssvc.c
@@ -276,15 +276,6 @@ struct tevent_req *mdscli_search_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->response_blob.spotlight_blob = talloc_array(
-		state,
-		uint8_t,
-		mdscli_ctx->max_fragment_size);
-	if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) {
-		return tevent_req_post(req, ev);
-	}
-	state->response_blob.size = mdscli_ctx->max_fragment_size;
-
 	subreq = dcerpc_mdssvc_cmd_send(state,
 					ev,
 					mdscli_ctx->bh,
@@ -457,15 +448,6 @@ struct tevent_req *mdscli_get_results_send(
 		return tevent_req_post(req, ev);
 	}
 
-	state->response_blob.spotlight_blob = talloc_array(
-		state,
-		uint8_t,
-		mdscli_ctx->max_fragment_size);
-	if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) {
-		return tevent_req_post(req, ev);
-	}
-	state->response_blob.size = mdscli_ctx->max_fragment_size;
-
 	subreq = dcerpc_mdssvc_cmd_send(state,
 					ev,
 					mdscli_ctx->bh,
@@ -681,15 +663,6 @@ struct tevent_req *mdscli_get_path_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->response_blob.spotlight_blob = talloc_array(
-		state,
-		uint8_t,
-		mdscli_ctx->max_fragment_size);
-	if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) {
-		return tevent_req_post(req, ev);
-	}
-	state->response_blob.size = mdscli_ctx->max_fragment_size;
-
 	subreq = dcerpc_mdssvc_cmd_send(state,
 					ev,
 					mdscli_ctx->bh,
@@ -852,15 +825,6 @@ struct tevent_req *mdscli_close_search_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 
-	state->response_blob.spotlight_blob = talloc_array(
-		state,
-		uint8_t,
-		mdscli_ctx->max_fragment_size);
-	if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) {
-		return tevent_req_post(req, ev);
-	}
-	state->response_blob.size = mdscli_ctx->max_fragment_size;
-
 	subreq = dcerpc_mdssvc_cmd_send(state,
 					ev,
 					mdscli_ctx->bh,
-- 
2.40.0


From 44a3c428219ad76c62f2b93294011a8e66262edc Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 20 Jun 2023 11:28:47 +0200
Subject: [PATCH 06/12] CVE-2023-34968: smbtorture: remove response blob
 allocation in mdssvc.c

This is alreay done by NDR for us.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source4/torture/rpc/mdssvc.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c
index 3689692f7de0..a16bd5b47e3a 100644
--- a/source4/torture/rpc/mdssvc.c
+++ b/source4/torture/rpc/mdssvc.c
@@ -536,13 +536,6 @@ static bool test_mdssvc_invalid_ph_cmd(struct torture_context *tctx,
 	request_blob.length = 0;
 	request_blob.size = 0;
 
-	response_blob.spotlight_blob = talloc_array(state,
-						    uint8_t,
-						    0);
-	torture_assert_not_null_goto(tctx, response_blob.spotlight_blob,
-				     ok, done, "dalloc_zero failed\n");
-	response_blob.size = 0;
-
 	status =  dcerpc_mdssvc_cmd(b,
 				    state,
 				    &ph,
@@ -632,13 +625,6 @@ static bool test_mdssvc_sl_unpack_loop(struct torture_context *tctx,
 	request_blob.size = sizeof(test_sl_unpack_loop_buf);
 	request_blob.length = sizeof(test_sl_unpack_loop_buf);
 
-	response_blob.spotlight_blob = talloc_array(state,
-						    uint8_t,
-						    0);
-	torture_assert_not_null_goto(tctx, response_blob.spotlight_blob,
-				     ok, done, "dalloc_zero failed\n");
-	response_blob.size = 0;
-
 	status = dcerpc_mdssvc_cmd(b,
 				   state,
 				   &state->ph,
@@ -764,11 +750,6 @@ static bool test_sl_dict_type_safety(struct torture_context *tctx,
 	torture_assert_goto(tctx, request_blob.length > 0,
 			    ok, done, "sl_pack failed\n");
 
-	response_blob.spotlight_blob = talloc_array(state, uint8_t, 0);
-	torture_assert_not_null_goto(tctx, response_blob.spotlight_blob,
-				     ok, done, "dalloc_zero failed\n");
-	response_blob.size = 0;
-
 	status = dcerpc_mdssvc_cmd(b,
 				   state,
 				   &state->ph,
@@ -926,13 +907,6 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx,
 				     ret, done, "dalloc_zero failed\n");
 	request_blob.size = max_fragment_size;
 
-	response_blob.spotlight_blob = talloc_array(state,
-						    uint8_t,
-						    max_fragment_size);
-	torture_assert_not_null_goto(tctx, response_blob.spotlight_blob,
-				     ret, done, "dalloc_zero failed\n");
-	response_blob.size = max_fragment_size;
-
 	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
 	torture_assert_goto(tctx, len != -1, ret, done, "sl_pack failed\n");
 
-- 
2.40.0


From 5ba4088a2d64eaa4dd34b47eff5fbdd286adc8b3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 20 Jun 2023 11:35:41 +0200
Subject: [PATCH 07/12] CVE-2023-34968: rpcclient: remove response blob
 allocation

This is alreay done by NDR for us.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpcclient/cmd_spotlight.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/source3/rpcclient/cmd_spotlight.c b/source3/rpcclient/cmd_spotlight.c
index 24db9893df63..64fe321089c4 100644
--- a/source3/rpcclient/cmd_spotlight.c
+++ b/source3/rpcclient/cmd_spotlight.c
@@ -144,13 +144,6 @@ static NTSTATUS cmd_mdssvc_fetch_properties(
 	}
 	request_blob.size = max_fragment_size;
 
-	response_blob.spotlight_blob = talloc_array(mem_ctx, uint8_t, max_fragment_size);
-	if (response_blob.spotlight_blob == NULL) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-	response_blob.size = max_fragment_size;
-
 	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
 	if (len == -1) {
 		status = NT_STATUS_INTERNAL_ERROR;
@@ -368,15 +361,6 @@ static NTSTATUS cmd_mdssvc_fetch_attributes(
 	}
 	request_blob.size = max_fragment_size;
 
-	response_blob.spotlight_blob = talloc_array(mem_ctx,
-						    uint8_t,
-						    max_fragment_size);
-	if (response_blob.spotlight_blob == NULL) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-	response_blob.size = max_fragment_size;
-
 	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
 	if (len == -1) {
 		status = NT_STATUS_INTERNAL_ERROR;
-- 
2.40.0


From 776b9beadfbd7f1a5e25569da4015010003a4b7a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 20 Jun 2023 11:42:10 +0200
Subject: [PATCH 08/12] CVE-2023-34968: mdssvc: remove response blob allocation

This is alreay done by NDR for us.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
index 2fca15cb8a8d..2fec2bb67251 100644
--- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
+++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
@@ -164,7 +164,6 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r)
 	struct auth_session_info *session_info =
 		dcesrv_call_session_info(dce_call);
 	bool ok;
-	char *rbuf;
 	struct mds_ctx *mds_ctx;
 	NTSTATUS status;
 
@@ -221,14 +220,6 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r)
 		return;
 	}
 
-	rbuf = talloc_zero_array(p->mem_ctx, char, r->in.max_fragment_size1);
-	if (rbuf == NULL) {
-		p->fault_state = DCERPC_FAULT_CANT_PERFORM;
-		return;
-	}
-	r->out.response_blob->spotlight_blob = (uint8_t *)rbuf;
-	r->out.response_blob->size = r->in.max_fragment_size1;
-
 	/* We currently don't use fragmentation at the mdssvc RPC layer */
 	*r->out.fragment = 0;
 
-- 
2.40.0


From f891847147a21de6521578a457e4a4c947538ccd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Tue, 20 Jun 2023 11:05:22 +0200
Subject: [PATCH 09/12] CVE-2023-34968: mdssvc: switch to doing an early return

Just reduce indentation of the code handling the success case. No change in
behaviour.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_server/mdssvc/mdssvc.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
index 72936a992890..92afa9e5fd49 100644
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -1817,19 +1817,21 @@ bool mds_dispatch(struct mds_ctx *mds_ctx,
 	}
 
 	ok = slcmd->function(mds_ctx, query, reply);
-	if (ok) {
-		DBG_DEBUG("%s", dalloc_dump(reply, 0));
-
-		len = sl_pack(reply,
-			      (char *)response_blob->spotlight_blob,
-			      response_blob->size);
-		if (len == -1) {
-			DBG_ERR("error packing Spotlight RPC reply\n");
-			ok = false;
-			goto cleanup;
-		}
-		response_blob->length = len;
+	if (!ok) {
+		goto cleanup;
+	}
+
+	DBG_DEBUG("%s", dalloc_dump(reply, 0));
+
+	len = sl_pack(reply,
+		      (char *)response_blob->spotlight_blob,
+		      response_blob->size);
+	if (len == -1) {
+		DBG_ERR("error packing Spotlight RPC reply\n");
+		ok = false;
+		goto cleanup;
 	}
+	response_blob->length = len;
 
 cleanup:
 	talloc_free(query);
-- 
2.40.0


From 6ebf8f8dd230393525c4fc1c4111772279fc66d4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 19 Jun 2023 18:16:57 +0200
Subject: [PATCH 10/12] CVE-2023-34968: mdssvc: introduce an allocating wrapper
 to sl_pack()

sl_pack_alloc() does the buffer allocation that previously all callers of
sl_pack() did themselves.

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_client/cli_mdssvc_util.c      | 80 +++++------------------
 source3/rpc_server/mdssvc/marshalling.c   | 35 ++++++++--
 source3/rpc_server/mdssvc/marshalling.h   |  9 ++-
 source3/rpc_server/mdssvc/mdssvc.c        | 18 ++---
 source3/rpc_server/mdssvc/mdssvc.h        |  5 +-
 source3/rpc_server/mdssvc/srv_mdssvc_nt.c |  5 +-
 source3/rpcclient/cmd_spotlight.c         | 32 ++-------
 source4/torture/rpc/mdssvc.c              | 24 ++-----
 8 files changed, 80 insertions(+), 128 deletions(-)

diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c
index 892a844e71a8..a39202d0c99e 100644
--- a/source3/rpc_client/cli_mdssvc_util.c
+++ b/source3/rpc_client/cli_mdssvc_util.c
@@ -42,7 +42,7 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx,
 	sl_array_t *scope_array = NULL;
 	double dval;
 	uint64_t uint64val;
-	ssize_t len;
+	NTSTATUS status;
 	int ret;
 
 	d = dalloc_new(mem_ctx);
@@ -209,23 +209,11 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(mem_ctx,
-					    uint8_t,
-					    ctx->max_fragment_size);
-	if (blob->spotlight_blob == NULL) {
-		TALLOC_FREE(d);
-		return NT_STATUS_NO_MEMORY;
-	}
-	blob->size = ctx->max_fragment_size;
-
-	len = sl_pack(d, (char *)blob->spotlight_blob, blob->size);
+	status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size);
 	TALLOC_FREE(d);
-	if (len == -1) {
-		return NT_STATUS_NO_MEMORY;
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
-
-	blob->length = len;
-	blob->size = len;
 	return NT_STATUS_OK;
 }
 
@@ -238,7 +226,7 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx,
 	uint64_t *uint64p = NULL;
 	sl_array_t *array = NULL;
 	sl_array_t *cmd_array = NULL;
-	ssize_t len;
+	NTSTATUS status;
 	int ret;
 
 	d = dalloc_new(mem_ctx);
@@ -293,23 +281,11 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(mem_ctx,
-					    uint8_t,
-					    ctx->max_fragment_size);
-	if (blob->spotlight_blob == NULL) {
-		TALLOC_FREE(d);
-		return NT_STATUS_NO_MEMORY;
-	}
-	blob->size = ctx->max_fragment_size;
-
-	len = sl_pack(d, (char *)blob->spotlight_blob, blob->size);
+	status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size);
 	TALLOC_FREE(d);
-	if (len == -1) {
-		return NT_STATUS_NO_MEMORY;
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
-
-	blob->length = len;
-	blob->size = len;
 	return NT_STATUS_OK;
 }
 
@@ -325,7 +301,7 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx,
 	sl_array_t *cmd_array = NULL;
 	sl_array_t *attr_array = NULL;
 	sl_cnids_t *cnids = NULL;
-	ssize_t len;
+	NTSTATUS status;
 	int ret;
 
 	d = dalloc_new(mem_ctx);
@@ -426,23 +402,11 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(mem_ctx,
-					    uint8_t,
-					    ctx->max_fragment_size);
-	if (blob->spotlight_blob == NULL) {
-		TALLOC_FREE(d);
-		return NT_STATUS_NO_MEMORY;
-	}
-	blob->size = ctx->max_fragment_size;
-
-	len = sl_pack(d, (char *)blob->spotlight_blob, blob->size);
+	status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size);
 	TALLOC_FREE(d);
-	if (len == -1) {
-		return NT_STATUS_NO_MEMORY;
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
-
-	blob->length = len;
-	blob->size = len;
 	return NT_STATUS_OK;
 }
 
@@ -455,7 +419,7 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx,
 	uint64_t *uint64p = NULL;
 	sl_array_t *array = NULL;
 	sl_array_t *cmd_array = NULL;
-	ssize_t len;
+	NTSTATUS status;
 	int ret;
 
 	d = dalloc_new(mem_ctx);
@@ -510,22 +474,10 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	blob->spotlight_blob = talloc_array(mem_ctx,
-					    uint8_t,
-					    ctx->max_fragment_size);
-	if (blob->spotlight_blob == NULL) {
-		TALLOC_FREE(d);
-		return NT_STATUS_NO_MEMORY;
-	}
-	blob->size = ctx->max_fragment_size;
-
-	len = sl_pack(d, (char *)blob->spotlight_blob, blob->size);
+	status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size);
 	TALLOC_FREE(d);
-	if (len == -1) {
-		return NT_STATUS_NO_MEMORY;
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
-
-	blob->length = len;
-	blob->size = len;
 	return NT_STATUS_OK;
 }
diff --git a/source3/rpc_server/mdssvc/marshalling.c b/source3/rpc_server/mdssvc/marshalling.c
index 441d41160f1a..34bfda5eca67 100644
--- a/source3/rpc_server/mdssvc/marshalling.c
+++ b/source3/rpc_server/mdssvc/marshalling.c
@@ -78,6 +78,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, const char *buf,
 			      ssize_t offset, size_t bufsize,
 			      int count, ssize_t toc_offset,
 			      int encoding);
+static ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize);
 
 /******************************************************************************
  * Wrapper functions for the *VAL macros with bound checking
@@ -1190,11 +1191,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query,
 	return offset;
 }
 
-/******************************************************************************
- * Global functions for packing und unpacking
- ******************************************************************************/
-
-ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize)
+static ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize)
 {
 	ssize_t result;
 	char *toc_buf;
@@ -1274,6 +1271,34 @@ ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize)
 	return len;
 }
 
+/******************************************************************************
+ * Global functions for packing und unpacking
+ ******************************************************************************/
+
+NTSTATUS sl_pack_alloc(TALLOC_CTX *mem_ctx,
+		       DALLOC_CTX *d,
+		       struct mdssvc_blob *b,
+		       size_t max_fragment_size)
+{
+	ssize_t len;
+
+	b->spotlight_blob = talloc_zero_array(mem_ctx,
+					      uint8_t,
+					      max_fragment_size);
+	if (b->spotlight_blob == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	len = sl_pack(d, (char *)b->spotlight_blob, max_fragment_size);
+	if (len == -1) {
+		return NT_STATUS_DATA_ERROR;
+	}
+
+	b->length = len;
+	b->size = len;
+	return NT_STATUS_OK;
+}
+
 bool sl_unpack(DALLOC_CTX *query, const char *buf, size_t bufsize)
 {
 	ssize_t result;
diff --git a/source3/rpc_server/mdssvc/marshalling.h b/source3/rpc_server/mdssvc/marshalling.h
index 086ca7406041..2cc1b44712cb 100644
--- a/source3/rpc_server/mdssvc/marshalling.h
+++ b/source3/rpc_server/mdssvc/marshalling.h
@@ -22,6 +22,9 @@
 #define _MDSSVC_MARSHALLING_H
 
 #include "dalloc.h"
+#include "libcli/util/ntstatus.h"
+#include "lib/util/data_blob.h"
+#include "librpc/gen_ndr/mdssvc.h"
 
 #define MAX_SL_FRAGMENT_SIZE 0xFFFFF
 
@@ -49,7 +52,11 @@ typedef struct {
  * Function declarations
  ******************************************************************************/
 
-extern ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize);
+extern NTSTATUS sl_pack_alloc(TALLOC_CTX *mem_ctx,
+			      DALLOC_CTX *d,
+			      struct mdssvc_blob *b,
+			      size_t max_fragment_size);
+
 extern bool sl_unpack(DALLOC_CTX *query, const char *buf, size_t bufsize);
 
 #endif
diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
index 92afa9e5fd49..0e62cba70382 100644
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -1745,11 +1745,11 @@ NTSTATUS mds_init_ctx(TALLOC_CTX *mem_ctx,
  **/
 bool mds_dispatch(struct mds_ctx *mds_ctx,
 		  struct mdssvc_blob *request_blob,
-		  struct mdssvc_blob *response_blob)
+		  struct mdssvc_blob *response_blob,
+		  size_t max_fragment_size)
 {
 	bool ok;
 	int ret;
-	ssize_t len;
 	DALLOC_CTX *query = NULL;
 	DALLOC_CTX *reply = NULL;
 	char *rpccmd;
@@ -1757,6 +1757,7 @@ bool mds_dispatch(struct mds_ctx *mds_ctx,
 	const struct smb_filename conn_basedir = {
 		.base_name = mds_ctx->conn->connectpath,
 	};
+	NTSTATUS status;
 
 	if (CHECK_DEBUGLVL(10)) {
 		const struct sl_query *slq;
@@ -1823,15 +1824,14 @@ bool mds_dispatch(struct mds_ctx *mds_ctx,
 
 	DBG_DEBUG("%s", dalloc_dump(reply, 0));
 
-	len = sl_pack(reply,
-		      (char *)response_blob->spotlight_blob,
-		      response_blob->size);
-	if (len == -1) {
-		DBG_ERR("error packing Spotlight RPC reply\n");
-		ok = false;
+	status = sl_pack_alloc(response_blob,
+			       reply,
+			       response_blob,
+			       max_fragment_size);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_ERR("sl_pack_alloc() failed\n");
 		goto cleanup;
 	}
-	response_blob->length = len;
 
 cleanup:
 	talloc_free(query);
diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h
index ff36b329f2b0..e1fc0709ab17 100644
--- a/source3/rpc_server/mdssvc/mdssvc.h
+++ b/source3/rpc_server/mdssvc/mdssvc.h
@@ -158,9 +158,10 @@ NTSTATUS mds_init_ctx(TALLOC_CTX *mem_ctx,
 		      const char *sharename,
 		      const char *path,
 		      struct mds_ctx **_mds_ctx);
-extern bool mds_dispatch(struct mds_ctx *query_ctx,
+extern bool mds_dispatch(struct mds_ctx *mds_ctx,
 			 struct mdssvc_blob *request_blob,
-			 struct mdssvc_blob *response_blob);
+			 struct mdssvc_blob *response_blob,
+			 size_t max_fragment_size);
 bool mds_add_result(struct sl_query *slq, const char *path);
 
 #endif /* _MDSSVC_H */
diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
index 2fec2bb67251..ae6a73605e19 100644
--- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
+++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
@@ -223,7 +223,10 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r)
 	/* We currently don't use fragmentation at the mdssvc RPC layer */
 	*r->out.fragment = 0;
 
-	ok = mds_dispatch(mds_ctx, &r->in.request_blob, r->out.response_blob);
+	ok = mds_dispatch(mds_ctx,
+			  &r->in.request_blob,
+			  r->out.response_blob,
+			  r->in.max_fragment_size1);
 	if (ok) {
 		*r->out.unkn9 = 0;
 	} else {
diff --git a/source3/rpcclient/cmd_spotlight.c b/source3/rpcclient/cmd_spotlight.c
index 64fe321089c4..ba3f61fd4b06 100644
--- a/source3/rpcclient/cmd_spotlight.c
+++ b/source3/rpcclient/cmd_spotlight.c
@@ -43,7 +43,6 @@ static NTSTATUS cmd_mdssvc_fetch_properties(
 	uint32_t unkn3;	     /* server always returns 0 ? */
 	struct mdssvc_blob request_blob;
 	struct mdssvc_blob response_blob;
-	ssize_t len;
 	uint32_t max_fragment_size = 64 * 1024;
 	DALLOC_CTX *d, *mds_reply;
 	uint64_t *uint64var;
@@ -137,20 +136,10 @@ static NTSTATUS cmd_mdssvc_fetch_properties(
 		goto done;
 	}
 
-	request_blob.spotlight_blob = talloc_array(mem_ctx, uint8_t, max_fragment_size);
-	if (request_blob.spotlight_blob == NULL) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-	request_blob.size = max_fragment_size;
-
-	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
-	if (len == -1) {
-		status = NT_STATUS_INTERNAL_ERROR;
+	status = sl_pack_alloc(mem_ctx, d, &request_blob, max_fragment_size);
+	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
-	request_blob.length = len;
-	request_blob.size = len;
 
 	status =  dcerpc_mdssvc_cmd(b, mem_ctx,
 				    &share_handle,
@@ -204,7 +193,6 @@ static NTSTATUS cmd_mdssvc_fetch_attributes(
 	uint32_t unkn3;	     /* server always returns 0 ? */
 	struct mdssvc_blob request_blob;
 	struct mdssvc_blob response_blob;
-	ssize_t len;
 	uint32_t max_fragment_size = 64 * 1024;
 	DALLOC_CTX *d, *mds_reply;
 	uint64_t *uint64var;
@@ -352,22 +340,10 @@ static NTSTATUS cmd_mdssvc_fetch_attributes(
 		goto done;
 	}
 
-	request_blob.spotlight_blob = talloc_array(mem_ctx,
-						   uint8_t,
-						   max_fragment_size);
-	if (request_blob.spotlight_blob == NULL) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto done;
-	}
-	request_blob.size = max_fragment_size;
-
-	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
-	if (len == -1) {
-		status = NT_STATUS_INTERNAL_ERROR;
+	status = sl_pack_alloc(mem_ctx, d, &request_blob, max_fragment_size);
+	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
-	request_blob.length = len;
-	request_blob.size = len;
 
 	status = dcerpc_mdssvc_cmd(b, mem_ctx,
 				   &share_handle,
diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c
index a16bd5b47e3a..afe7068ee1bb 100644
--- a/source4/torture/rpc/mdssvc.c
+++ b/source4/torture/rpc/mdssvc.c
@@ -744,11 +744,9 @@ static bool test_sl_dict_type_safety(struct torture_context *tctx,
 				     ok, done, "dalloc_new failed\n");
 	request_blob.size = 64 * 1024;
 
-	request_blob.length = sl_pack(d,
-				      (char *)request_blob.spotlight_blob,
-				      request_blob.size);
-	torture_assert_goto(tctx, request_blob.length > 0,
-			    ok, done, "sl_pack failed\n");
+	status = sl_pack_alloc(tctx, d, &request_blob, 64 * 1024);
+	torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
+					"sl_pack_alloc() failed\n");
 
 	status = dcerpc_mdssvc_cmd(b,
 				   state,
@@ -835,7 +833,6 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx,
 	const char *path_type = NULL;
 	uint64_t ino64;
 	NTSTATUS status;
-	ssize_t len;
 	int ret;
 	bool ok = true;
 
@@ -900,18 +897,9 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx,
 	ret = dalloc_add(array, cnids, sl_cnids_t);
 	torture_assert_goto(tctx, ret == 0, ret, done, "dalloc_add failed\n");
 
-	request_blob.spotlight_blob = talloc_array(state,
-						   uint8_t,
-						   max_fragment_size);
-	torture_assert_not_null_goto(tctx, request_blob.spotlight_blob,
-				     ret, done, "dalloc_zero failed\n");
-	request_blob.size = max_fragment_size;
-
-	len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size);
-	torture_assert_goto(tctx, len != -1, ret, done, "sl_pack failed\n");
-
-	request_blob.length = len;
-	request_blob.size = len;
+	status = sl_pack_alloc(tctx, d, &request_blob, max_fragment_size);
+	torture_assert_ntstatus_ok_goto(tctx, status, ok, done,
+					"sl_pack_alloc() failed\n");
 
 	status =  dcerpc_mdssvc_cmd(b,
 				    state,
-- 
2.40.0


From 69146a3fd4daa48e40cb61316f65475f3128d619 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 17 Jun 2023 13:53:27 +0200
Subject: [PATCH 11/12] CVE-2023-34968: mdscli: return share relative paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The next commit will change the Samba Spotlight server to return absolute paths
that start with the sharename as "/SHARENAME/..." followed by the share path
relative appended.

So given a share

  [spotlight]
    path = /foo/bar
    spotlight = yes

and a file inside this share with a full path of

  /foo/bar/dir/file

previously a search that matched this file would returns the absolute
server-side pato of the file, ie

  /foo/bar/dir/file

This will be change to

  /spotlight/dir/file

As currently the mdscli library and hence the mdsearch tool print out these
paths returned from the server, we have to change the output to accomodate these
fake paths. The only way to do this sensibly is by makeing the paths relative to
the containing share, so just

  dir/file

in the example above.

The client learns about the share root path prefix – real server-side of fake in
the future – in an initial handshake in the "share_path" out argument of the
mdssvc_open() RPC call, so the client can use this path to convert the absolute
path to relative.

There is however an additional twist: the macOS Spotlight server prefixes this
absolute path with another prefix, typically "/System/Volumes/Data", so in the
example above the full path for the same search would be

  /System/Volumes/Data/foo/bar/dir/file

So macOS does return the full server-side path too, just prefixed with an
additional path. This path prefixed can be queried by the client in the
mdssvc_cmd() RPC call with an Spotlight command of "fetchPropertiesForContext:"
and the path is returned in a dictionary with key "kMDSStorePathScopes". Samba
just returns "/" for this.

Currently the mdscli library doesn't issue this Spotlight RPC
request (fetchPropertiesForContext), so this is added in this commit. In the
end, all search result paths are stripped of the combined prefix

  kMDSStorePathScopes + share_path (from mdssvc_open).

eg

  kMDSStorePathScopes = /System/Volumes/Data
  share_path = /foo/bar
  search result = /System/Volumes/Data/foo/bar/dir/file
  relative path returned by mdscli = dir/file

Makes sense? :)

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 python/samba/tests/blackbox/mdsearch.py |   8 +-
 python/samba/tests/dcerpc/mdssvc.py     |  26 ++--
 source3/rpc_client/cli_mdssvc.c         | 155 +++++++++++++++++++++++-
 source3/rpc_client/cli_mdssvc_private.h |   4 +
 source3/rpc_client/cli_mdssvc_util.c    |  68 +++++++++++
 source3/rpc_client/cli_mdssvc_util.h    |   4 +
 6 files changed, 245 insertions(+), 20 deletions(-)

diff --git a/python/samba/tests/blackbox/mdsearch.py b/python/samba/tests/blackbox/mdsearch.py
index c9156ae6e0e4..c8e75661f151 100644
--- a/python/samba/tests/blackbox/mdsearch.py
+++ b/python/samba/tests/blackbox/mdsearch.py
@@ -76,10 +76,7 @@ testfiles = [
         self.t.start()
         time.sleep(1)
 
-        pipe = mdssvc.mdssvc('ncacn_np:fileserver[/pipe/mdssvc]', self.get_loadparm())
-        conn = mdscli.conn(pipe, 'spotlight', '/foo')
-        self.sharepath = conn.sharepath()
-        conn.disconnect(pipe)
+        self.sharepath = os.environ["LOCAL_PATH"]
 
         for file in testfiles:
             f = open("%s/%s" % (self.sharepath, file), "w")
@@ -126,5 +123,4 @@ testfiles = [
         output = self.check_output("mdsearch --configfile=%s -U %s%%%s fileserver spotlight '*==\"samba*\"'" % (config, username, password))
 
         actual = output.decode('utf-8').splitlines()
-        expected = ["%s/%s" % (self.sharepath, file) for file in testfiles]
-        self.assertEqual(expected, actual)
+        self.assertEqual(testfiles, actual)
diff --git a/python/samba/tests/dcerpc/mdssvc.py b/python/samba/tests/dcerpc/mdssvc.py
index b0df509ddc79..5002e5d26d64 100644
--- a/python/samba/tests/dcerpc/mdssvc.py
+++ b/python/samba/tests/dcerpc/mdssvc.py
@@ -84,10 +84,11 @@ testfiles = [
         self.t = threading.Thread(target=MdssvcTests.http_server, args=(self,))
         self.t.setDaemon(True)
         self.t.start()
+        self.sharepath = os.environ["LOCAL_PATH"]
         time.sleep(1)
 
         conn = mdscli.conn(self.pipe, 'spotlight', '/foo')
-        self.sharepath = conn.sharepath()
+        self.fakepath = conn.sharepath()
         conn.disconnect(self.pipe)
 
         for file in testfiles:
@@ -105,12 +106,11 @@ testfiles = [
         self.server.serve_forever()
 
     def run_test(self, query, expect, json_in, json_out):
-        expect = [s.replace("%BASEPATH%", self.sharepath) for s in expect]
         self.server.json_in = json_in.replace("%BASEPATH%", self.sharepath)
         self.server.json_out = json_out.replace("%BASEPATH%", self.sharepath)
 
         self.conn = mdscli.conn(self.pipe, 'spotlight', '/foo')
-        search = self.conn.search(self.pipe, query, self.sharepath)
+        search = self.conn.search(self.pipe, query, self.fakepath)
 
         # Give it some time, the get_results() below returns immediately
         # what's available, so if we ask to soon, we might get back no results
@@ -141,7 +141,7 @@ testfiles = [
             ]
           }
         }'''
-        exp_results = ["%BASEPATH%/foo", "%BASEPATH%/bar"]
+        exp_results = ["foo", "bar"]
         self.run_test('*=="samba*"', exp_results, exp_json_query, fake_json_response)
 
     def test_mdscli_search_escapes(self):
@@ -181,14 +181,14 @@ testfiles = [
           }
         }'''
         exp_results = [
-            r"%BASEPATH%/x+x",
-            r"%BASEPATH%/x*x",
-            r"%BASEPATH%/x=x",
-            r"%BASEPATH%/x'x",
-            r"%BASEPATH%/x?x",
-            r"%BASEPATH%/x x",
-            r"%BASEPATH%/x(x",
-            "%BASEPATH%/x\"x",
-            r"%BASEPATH%/x\x",
+            r"x+x",
+            r"x*x",
+            r"x=x",
+            r"x'x",
+            r"x?x",
+            r"x x",
+            r"x(x",
+            "x\"x",
+            r"x\x",
         ]
         self.run_test(sl_query, exp_results, exp_json_query, fake_json_response)
diff --git a/source3/rpc_client/cli_mdssvc.c b/source3/rpc_client/cli_mdssvc.c
index 07c19b51dd49..03aed61c00cf 100644
--- a/source3/rpc_client/cli_mdssvc.c
+++ b/source3/rpc_client/cli_mdssvc.c
@@ -43,10 +43,12 @@ char *mdscli_get_basepath(TALLOC_CTX *mem_ctx,
 struct mdscli_connect_state {
 	struct tevent_context *ev;
 	struct mdscli_ctx *mdscli_ctx;
+	struct mdssvc_blob response_blob;
 };
 
 static void mdscli_connect_open_done(struct tevent_req *subreq);
 static void mdscli_connect_unknown1_done(struct tevent_req *subreq);
+static void mdscli_connect_fetch_props_done(struct tevent_req *subreq);
 
 struct tevent_req *mdscli_connect_send(TALLOC_CTX *mem_ctx,
 				       struct tevent_context *ev,
@@ -111,6 +113,7 @@ static void mdscli_connect_open_done(struct tevent_req *subreq)
 	struct mdscli_connect_state *state = tevent_req_data(
 		req, struct mdscli_connect_state);
 	struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx;
+	size_t share_path_len;
 	NTSTATUS status;
 
 	status = dcerpc_mdssvc_open_recv(subreq, state);
@@ -120,6 +123,18 @@ static void mdscli_connect_open_done(struct tevent_req *subreq)
 		return;
 	}
 
+	share_path_len = strlen(mdscli_ctx->mdscmd_open.share_path);
+	if (share_path_len < 1 || share_path_len > UINT16_MAX) {
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+	mdscli_ctx->mdscmd_open.share_path_len = share_path_len;
+
+	if (mdscli_ctx->mdscmd_open.share_path[share_path_len-1] == '/') {
+		mdscli_ctx->mdscmd_open.share_path[share_path_len-1] = '\0';
+		mdscli_ctx->mdscmd_open.share_path_len--;
+	}
+
 	subreq = dcerpc_mdssvc_unknown1_send(
 			state,
 			state->ev,
@@ -146,6 +161,8 @@ static void mdscli_connect_unknown1_done(struct tevent_req *subreq)
 		subreq, struct tevent_req);
 	struct mdscli_connect_state *state = tevent_req_data(
 		req, struct mdscli_connect_state);
+	struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx;
+	struct mdssvc_blob request_blob;
 	NTSTATUS status;
 
 	status = dcerpc_mdssvc_unknown1_recv(subreq, state);
@@ -154,6 +171,108 @@ static void mdscli_connect_unknown1_done(struct tevent_req *subreq)
 		return;
 	}
 
+	status = mdscli_blob_fetch_props(state,
+					 state->mdscli_ctx,
+					 &request_blob);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
+	subreq = dcerpc_mdssvc_cmd_send(state,
+					state->ev,
+					mdscli_ctx->bh,
+					&mdscli_ctx->ph,
+					0,
+					mdscli_ctx->dev,
+					mdscli_ctx->mdscmd_open.unkn2,
+					0,
+					mdscli_ctx->flags,
+					request_blob,
+					0,
+					mdscli_ctx->max_fragment_size,
+					1,
+					mdscli_ctx->max_fragment_size,
+					0,
+					0,
+					&mdscli_ctx->mdscmd_cmd.fragment,
+					&state->response_blob,
+					&mdscli_ctx->mdscmd_cmd.unkn9);
+	if (tevent_req_nomem(subreq, req)) {
+		return;
+	}
+	tevent_req_set_callback(subreq, mdscli_connect_fetch_props_done, req);
+	mdscli_ctx->async_pending++;
+	return;
+}
+
+static void mdscli_connect_fetch_props_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
+	struct mdscli_connect_state *state = tevent_req_data(
+		req, struct mdscli_connect_state);
+	struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx;
+	DALLOC_CTX *d = NULL;
+	sl_array_t *path_scope_array = NULL;
+	char *path_scope = NULL;
+	NTSTATUS status;
+	bool ok;
+
+	status = dcerpc_mdssvc_cmd_recv(subreq, state);
+	TALLOC_FREE(subreq);
+	state->mdscli_ctx->async_pending--;
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
+	d = dalloc_new(state);
+	if (tevent_req_nomem(d, req)) {
+		return;
+	}
+
+	ok = sl_unpack(d,
+		       (char *)state->response_blob.spotlight_blob,
+		       state->response_blob.length);
+	if (!ok) {
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+
+	path_scope_array = dalloc_value_for_key(d,
+						"DALLOC_CTX", 0,
+						"kMDSStorePathScopes",
+						"sl_array_t");
+	if (path_scope_array == NULL) {
+		DBG_ERR("Missing kMDSStorePathScopes\n");
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+
+	path_scope = dalloc_get(path_scope_array, "char *", 0);
+	if (path_scope == NULL) {
+		DBG_ERR("Missing path in kMDSStorePathScopes\n");
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+
+	mdscli_ctx->path_scope_len = strlen(path_scope);
+	if (mdscli_ctx->path_scope_len < 1 ||
+	    mdscli_ctx->path_scope_len > UINT16_MAX)
+	{
+		DBG_ERR("Bad path_scope: %s\n", path_scope);
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+		return;
+	}
+	mdscli_ctx->path_scope = talloc_strdup(mdscli_ctx, path_scope);
+	if (tevent_req_nomem(mdscli_ctx->path_scope, req)) {
+		return;
+	}
+
+	if (mdscli_ctx->path_scope[mdscli_ctx->path_scope_len-1] == '/') {
+		mdscli_ctx->path_scope[mdscli_ctx->path_scope_len-1] = '\0';
+		mdscli_ctx->path_scope_len--;
+	}
+
 	tevent_req_done(req);
 }
 
@@ -697,7 +816,10 @@ static void mdscli_get_path_done(struct tevent_req *subreq)
 	struct mdscli_get_path_state *state = tevent_req_data(
 		req, struct mdscli_get_path_state);
 	DALLOC_CTX *d = NULL;
+	size_t pathlen;
+	size_t prefixlen;
 	char *path = NULL;
+	const char *p = NULL;
 	NTSTATUS status;
 	bool ok;
 
@@ -732,7 +854,38 @@ static void mdscli_get_path_done(struct tevent_req *subreq)
 		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
 		return;
 	}
-	state->path = talloc_move(state, &path);
+
+	/* Path is prefixed by /PATHSCOPE/SHARENAME/, strip it */
+	pathlen = strlen(path);
+
+	/*
+	 * path_scope_len and share_path_len are already checked to be smaller
+	 * then UINT16_MAX so this can't overflow
+	 */
+	prefixlen = state->mdscli_ctx->path_scope_len
+		+ state->mdscli_ctx->mdscmd_open.share_path_len;
+
+	if (pathlen < prefixlen) {
+		DBG_DEBUG("Bad path: %s\n", path);
+		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+		return;
+	}
+
+	p = path + prefixlen;
+	while (*p == '/') {
+		p++;
+	}
+	if (*p == '\0') {
+		DBG_DEBUG("Bad path: %s\n", path);
+		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+		return;
+	}
+
+	state->path = talloc_strdup(state, p);
+	if (state->path == NULL) {
+		tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
+		return;
+	}
 	DBG_DEBUG("path: %s\n", state->path);
 
 	tevent_req_done(req);
diff --git a/source3/rpc_client/cli_mdssvc_private.h b/source3/rpc_client/cli_mdssvc_private.h
index 031af85bf583..77f300c09ccc 100644
--- a/source3/rpc_client/cli_mdssvc_private.h
+++ b/source3/rpc_client/cli_mdssvc_private.h
@@ -42,6 +42,7 @@ struct mdscli_ctx {
 	/* cmd specific or unknown fields */
 	struct {
 		char share_path[1025];
+		size_t share_path_len;
 		uint32_t unkn2;
 		uint32_t unkn3;
 	} mdscmd_open;
@@ -56,6 +57,9 @@ struct mdscli_ctx {
 	struct {
 		uint32_t status;
 	} mdscmd_close;
+
+	char *path_scope;
+	size_t path_scope_len;
 };
 
 struct mdscli_search_ctx {
diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c
index a39202d0c99e..1eaaca715a80 100644
--- a/source3/rpc_client/cli_mdssvc_util.c
+++ b/source3/rpc_client/cli_mdssvc_util.c
@@ -28,6 +28,74 @@
 #include "rpc_server/mdssvc/dalloc.h"
 #include "rpc_server/mdssvc/marshalling.h"
 
+NTSTATUS mdscli_blob_fetch_props(TALLOC_CTX *mem_ctx,
+				 struct mdscli_ctx *ctx,
+				 struct mdssvc_blob *blob)
+{
+	DALLOC_CTX *d = NULL;
+	uint64_t *uint64p = NULL;
+	sl_array_t *array = NULL;
+	sl_array_t *cmd_array = NULL;
+	NTSTATUS status;
+	int ret;
+
+	d = dalloc_new(mem_ctx);
+	if (d == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	array = dalloc_zero(d, sl_array_t);
+	if (array == NULL) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = dalloc_add(d, array, sl_array_t);
+	if (ret != 0) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	cmd_array = dalloc_zero(d, sl_array_t);
+	if (cmd_array == NULL) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = dalloc_add(array, cmd_array, sl_array_t);
+	if (ret != 0) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = dalloc_stradd(cmd_array, "fetchPropertiesForContext:");
+	if (ret != 0) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	uint64p = talloc_zero_array(cmd_array, uint64_t, 2);
+	if (uint64p == NULL) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	talloc_set_name(uint64p, "uint64_t *");
+
+	ret = dalloc_add(cmd_array, uint64p, uint64_t *);
+	if (ret != 0) {
+		TALLOC_FREE(d);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size);
+	TALLOC_FREE(d);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+	return NT_STATUS_OK;
+}
+
 NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx,
 			    struct mdscli_search_ctx *search,
 			    struct mdssvc_blob *blob)
diff --git a/source3/rpc_client/cli_mdssvc_util.h b/source3/rpc_client/cli_mdssvc_util.h
index 7a98c8545267..3f324758c707 100644
--- a/source3/rpc_client/cli_mdssvc_util.h
+++ b/source3/rpc_client/cli_mdssvc_util.h
@@ -21,6 +21,10 @@
 #ifndef _MDSCLI_UTIL_H_
 #define _MDSCLI_UTIL_H_
 
+NTSTATUS mdscli_blob_fetch_props(TALLOC_CTX *mem_ctx,
+				 struct mdscli_ctx *ctx,
+				 struct mdssvc_blob *blob);
+
 NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx,
 			    struct mdscli_search_ctx *search,
 			    struct mdssvc_blob *blob);
-- 
2.40.0


From c58882e5faefd0af1203adfaf4ef5439db7a4596 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 5 Jun 2023 18:02:20 +0200
Subject: [PATCH 12/12] CVE-2023-34968: mdssvc: return a fake share path

Instead of returning the real server-side absolute path of shares and search
results, return a fake absolute path replacing the path of the share with the
share name, iow for a share "test" with a server-side path of "/foo/bar", we
previously returned

  /foo/bar and
  /foo/bar/search/result

and now return

  /test and
  /test/search/result

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/rpc_server/mdssvc/mdssvc.c        | 61 +++++++++++++++++++++--
 source3/rpc_server/mdssvc/mdssvc.h        |  1 +
 source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 18 +++++--
 3 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
index 0e62cba70382..c616bdfa53bb 100644
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -520,11 +520,14 @@ static bool inode_map_add(struct sl_query *slq,
 bool mds_add_result(struct sl_query *slq, const char *path)
 {
 	struct smb_filename *smb_fname = NULL;
+	const char *relative = NULL;
+	char *fake_path = NULL;
 	struct stat_ex sb;
 	uint32_t attr;
 	uint64_t ino64;
 	int result;
 	NTSTATUS status;
+	bool sub;
 	bool ok;
 
 	/*
@@ -611,6 +614,17 @@ bool mds_add_result(struct sl_query *slq, const char *path)
 		}
 	}
 
+	sub = subdir_of(slq->mds_ctx->spath,
+			slq->mds_ctx->spath_len,
+			path,
+			&relative);
+	if (!sub) {
+		DBG_ERR("[%s] is not inside [%s]\n",
+			path, slq->mds_ctx->spath);
+		slq->state = SLQ_STATE_ERROR;
+		return false;
+	}
+
 	/*
 	 * Add inode number and filemeta to result set, this is what
 	 * we return as part of the result set of a query
@@ -623,18 +637,30 @@ bool mds_add_result(struct sl_query *slq, const char *path)
 		slq->state = SLQ_STATE_ERROR;
 		return false;
 	}
+
+	fake_path = talloc_asprintf(slq,
+				    "/%s/%s",
+				    slq->mds_ctx->sharename,
+				    relative);
+	if (fake_path == NULL) {
+		slq->state = SLQ_STATE_ERROR;
+		return false;
+	}
+
 	ok = add_filemeta(slq->mds_ctx,
 			  slq->reqinfo,
 			  slq->query_results->fm_array,
-			  path,
+			  fake_path,
 			  &sb);
 	if (!ok) {
 		DBG_ERR("add_filemeta error\n");
+		TALLOC_FREE(fake_path);
 		slq->state = SLQ_STATE_ERROR;
 		return false;
 	}
 
-	ok = inode_map_add(slq, ino64, path, &sb);
+	ok = inode_map_add(slq, ino64, fake_path, &sb);
+	TALLOC_FREE(fake_path);
 	if (!ok) {
 		DEBUG(1, ("inode_map_add error\n"));
 		slq->state = SLQ_STATE_ERROR;
@@ -841,6 +867,32 @@ static void slq_close_timer(struct tevent_context *ev,
 	}
 }
 
+/**
+ * Translate a fake scope from the client like /sharename/dir
+ * to the real server-side path, replacing the "/sharename" part
+ * with the absolute server-side path of the share.
+ **/
+static bool mdssvc_real_scope(struct sl_query *slq, const char *fake_scope)
+{
+	size_t sname_len = strlen(slq->mds_ctx->sharename);
+	size_t fake_scope_len = strlen(fake_scope);
+
+	if (fake_scope_len < sname_len + 1) {
+		DBG_ERR("Short scope [%s] for share [%s]\n",
+			fake_scope, slq->mds_ctx->sharename);
+		return false;
+	}
+
+	slq->path_scope = talloc_asprintf(slq,
+					  "%s%s",
+					  slq->mds_ctx->spath,
+					  fake_scope + sname_len + 1);
+	if (slq->path_scope == NULL) {
+		return false;
+	}
+	return true;
+}
+
 /**
  * Begin a search query
  **/
@@ -953,8 +1005,8 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx,
 		goto error;
 	}
 
-	slq->path_scope = talloc_strdup(slq, scope);
-	if (slq->path_scope == NULL) {
+	ok = mdssvc_real_scope(slq, scope);
+	if (!ok) {
 		goto error;
 	}
 
@@ -1675,6 +1727,7 @@ NTSTATUS mds_init_ctx(TALLOC_CTX *mem_ctx,
 		status = NT_STATUS_NO_MEMORY;
 		goto error;
 	}
+	mds_ctx->spath_len = strlen(path);
 
 	mds_ctx->snum = snum;
 	mds_ctx->pipe_session_info = session_info;
diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h
index e1fc0709ab17..3b2ce250f1f1 100644
--- a/source3/rpc_server/mdssvc/mdssvc.h
+++ b/source3/rpc_server/mdssvc/mdssvc.h
@@ -127,6 +127,7 @@ struct mds_ctx {
 	int snum;
 	const char *sharename;
 	const char *spath;
+	size_t spath_len;
 	struct connection_struct *conn;
 	struct sl_query *query_list;     /* list of active queries */
 	struct db_context *ino_path_map; /* dbwrap rbt for storing inode->path mappings */
diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
index ae6a73605e19..c77e71855211 100644
--- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
+++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c
@@ -81,6 +81,7 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r)
 		loadparm_s3_global_substitution();
 	int snum;
 	char *outpath = discard_const_p(char, r->out.share_path);
+	char *fake_path = NULL;
 	char *path;
 	NTSTATUS status;
 
@@ -98,12 +99,21 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r)
 
 	path = lp_path(talloc_tos(), lp_sub, snum);
 	if (path == NULL) {
-		DBG_ERR("Couldn't create policy handle for %s\n",
+		DBG_ERR("Couldn't create path for %s\n",
 			r->in.share_name);
 		p->fault_state = DCERPC_FAULT_CANT_PERFORM;
 		return;
 	}
 
+	fake_path = talloc_asprintf(p->mem_ctx, "/%s", r->in.share_name);
+	if (fake_path == NULL) {
+		DBG_ERR("Couldn't create fake share path for %s\n",
+			r->in.share_name);
+		talloc_free(path);
+		p->fault_state = DCERPC_FAULT_CANT_PERFORM;
+		return;
+	}
+
 	status = create_mdssvc_policy_handle(p->mem_ctx, p,
 					     snum,
 					     r->in.share_name,
@@ -112,18 +122,20 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r)
 	if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_VOLUME)) {
 		ZERO_STRUCTP(r->out.handle);
 		talloc_free(path);
+		talloc_free(fake_path);
 		return;
 	}
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_ERR("Couldn't create policy handle for %s\n",
 			r->in.share_name);
 		talloc_free(path);
+		talloc_free(fake_path);
 		p->fault_state = DCERPC_FAULT_CANT_PERFORM;
 		return;
 	}
 
-	strlcpy(outpath, path, 1024);
-	talloc_free(path);
+	strlcpy(outpath, fake_path, 1024);
+	talloc_free(fake_path);
 	return;
 }
 
-- 
2.40.0