Attachment 17932 Details for Bug 15388 – Patch for master

[patch] Patch for master

CVE-2023-34968-server_path-master-02.patch (text/plain), 50.33 KB, created by Ralph Böhme on 2023-06-21 13:30:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2023-06-21 13:30:18 UTC

Size: 50.33 KB

patch

obsolete

>From 5b1b58411578954ef0f8352b0c2af26597ada306 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 6 Jun 2023 15:17:26 +0200
>Subject: [PATCH 01/11] 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>
>---
> 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 7dd3c84713f1..a6d09a43b9c8 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"));
>@@ -617,7 +621,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;
>@@ -1340,29 +1344,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 821843128055dec02624575b2a079e61366022a1 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 17 Jun 2023 13:39:55 +0200
>Subject: [PATCH 02/11] 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>
>---
> 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 a6d09a43b9c8..9c23ef95753b 100644
>--- a/source3/rpc_server/mdssvc/mdssvc.c
>+++ b/source3/rpc_server/mdssvc/mdssvc.c
>@@ -730,6 +730,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 d579af824048931b6d0356dead73b349b7a488f2 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 19 Jun 2023 17:14:38 +0200
>Subject: [PATCH 03/11] 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>
>---
> 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 a699cd4cfaa946f0e2f23b7cbe2c0ddf81b949c2 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 19 Jun 2023 18:28:41 +0200
>Subject: [PATCH 04/11] 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>
>---
> 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 046d37135cbc..474d7c0b1500 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 e79eb95822a11291eb97a2da1941171503cb10f8 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 20 Jun 2023 11:28:47 +0200
>Subject: [PATCH 05/11] 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>
>---
> 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 fb02565e9ff2..d17880e94663 100644
>--- a/source4/torture/rpc/mdssvc.c
>+++ b/source4/torture/rpc/mdssvc.c
>@@ -544,13 +544,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,
>@@ -644,13 +637,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,
>@@ -776,11 +762,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,
>@@ -942,13 +923,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 7c805fc11ecdb8a65af8eba6cbf647a1fb1778fe Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 20 Jun 2023 11:35:41 +0200
>Subject: [PATCH 06/11] 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>
>---
> 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 b30d0ee2ab0525f6e77cdfcabec7150574ccf86d Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 20 Jun 2023 11:42:10 +0200
>Subject: [PATCH 07/11] 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>
>---
> 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 1b4948105aa74c330e08a6ef1f1d120d05a2a405 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 20 Jun 2023 11:05:22 +0200
>Subject: [PATCH 08/11] 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>
>---
> 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 9c23ef95753b..070503f9eeb8 100644
>--- a/source3/rpc_server/mdssvc/mdssvc.c
>+++ b/source3/rpc_server/mdssvc/mdssvc.c
>@@ -1804,19 +1804,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 1da61e7520527676d903d897432448cd13be1828 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 19 Jun 2023 18:16:57 +0200
>Subject: [PATCH 09/11] 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>
>---
> 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 d794ba158389..5f866d7fb6ea 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 070503f9eeb8..9b814f51fc36 100644
>--- a/source3/rpc_server/mdssvc/mdssvc.c
>+++ b/source3/rpc_server/mdssvc/mdssvc.c
>@@ -1732,11 +1732,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;
>@@ -1744,6 +1744,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;
>@@ -1810,15 +1811,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 d17880e94663..213ff32ba62a 100644
>--- a/source4/torture/rpc/mdssvc.c
>+++ b/source4/torture/rpc/mdssvc.c
>@@ -756,11 +756,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,
>@@ -851,7 +849,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;
> 
>@@ -916,18 +913,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 8cd70d3509bd39364626ecfd72e4907d76d603be Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 17 Jun 2023 13:53:27 +0200
>Subject: [PATCH 10/11] 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>
>---
> python/samba/tests/blackbox/mdsearch.py |   8 +-
> python/samba/tests/dcerpc/mdssvc.py     |  26 ++---
> source3/rpc_client/cli_mdssvc.c         | 143 +++++++++++++++++++++++-
> 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, 233 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 474d7c0b1500..995ed012b770 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) {
>+		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,106 @@ 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) {
>+		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);
> }
> 
>@@ -698,6 +815,7 @@ static void mdscli_get_path_done(struct tevent_req *subreq)
> 		req, struct mdscli_get_path_state);
> 	DALLOC_CTX *d = NULL;
> 	char *path = NULL;
>+	const char *p = NULL;
> 	NTSTATUS status;
> 	bool ok;
> 
>@@ -732,7 +850,30 @@ 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 /SHARENAME/, strip it */
>+	if (strlen(path) < state->mdscli_ctx->mdscmd_open.share_path_len) {
>+		DBG_DEBUG("Bad path: %s", path);
>+		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
>+		return;
>+	}
>+
>+	p = path + state->mdscli_ctx->mdscmd_open.share_path_len
>+		+ state->mdscli_ctx->path_scope_len;
>+	while (*p == '/') {
>+		p++;
>+	}
>+	if (*p == '\0') {
>+		DBG_DEBUG("Bad path: %s", 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 5f736bf6df7ea3b9378d934b242780ac5845a193 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 5 Jun 2023 18:02:20 +0200
>Subject: [PATCH 11/11] 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
>previosly returns
>
>  /foo/bar and
>  /foo/bar/search/result
>
>now return
>
>  /test and
>  /test/search/result
>
>This also changes the mdssvc RPC client to return relative paths instead of the
>previous absolute paths received from the server.
>
>To cope with the changes the relevant mdssvc tests have been adjusted to work
>with the fake paths.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> source3/rpc_server/mdssvc/mdssvc.c        | 60 +++++++++++++++++++++--
> source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 18 +++++--
> 2 files changed, 71 insertions(+), 7 deletions(-)
>
>diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
>index 9b814f51fc36..2ac697e23763 100644
>--- a/source3/rpc_server/mdssvc/mdssvc.c
>+++ b/source3/rpc_server/mdssvc/mdssvc.c
>@@ -520,10 +520,13 @@ 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;
> 	uint64_t ino64;
> 	int result;
> 	NTSTATUS status;
>+	bool sub;
> 	bool ok;
> 
> 	/*
>@@ -598,6 +601,17 @@ bool mds_add_result(struct sl_query *slq, const char *path)
> 		}
> 	}
> 
>+	sub = subdir_of(slq->mds_ctx->spath,
>+			strlen(slq->mds_ctx->spath),
>+			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
>@@ -610,18 +624,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;
>@@ -828,6 +854,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
>  **/
>@@ -940,8 +992,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;
> 	}
> 
>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
>

Flags:

slow: ci-passed+

Actions: View

Attachments on bug 15388: 17912 | 17918 | 17921 | 17932 | 17933 | 17936 | 17937 | 17945 | 17946 | 17952