samba/backport-0010-CVE-2023-34968.patch

From a5c570e262911874e43e82de601d809aa5b1b729 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 17 Jun 2023 13:53:27 +0200
Subject: [PATCH 16/28] 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>

Conflict: NA
Reference: https://download.samba.org/pub/samba/patches/security/samba-4.17.10-security-2023-07-19.patch
---
 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 c9156ae6e0e..c8e75661f15 100644
--- a/python/samba/tests/blackbox/mdsearch.py
+++ b/python/samba/tests/blackbox/mdsearch.py
@@ -76,10 +76,7 @@ class MdfindBlackboxTests(BlackboxTestCase):
         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 @@ class MdfindBlackboxTests(BlackboxTestCase):
         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 b0df509ddc7..5002e5d26d6 100644
--- a/python/samba/tests/dcerpc/mdssvc.py
+++ b/python/samba/tests/dcerpc/mdssvc.py
@@ -84,10 +84,11 @@ class MdssvcTests(RpcInterfaceTestCase):
         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 @@ class MdssvcTests(RpcInterfaceTestCase):
         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 @@ class MdssvcTests(RpcInterfaceTestCase):
             ]
           }
         }'''
-        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 @@ class MdssvcTests(RpcInterfaceTestCase):
           }
         }'''
         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 474d7c0b150..753bc2e52ed 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 031af85bf58..77f300c09cc 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 a39202d0c99..1eaaca715a8 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 7a98c854526..3f324758c70 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.34.1