Attachment 17921 Details for Bug 15388 – Patch for master

[patch] Patch for master

CVE-2023-34968-server_path-master-01.patch (text/plain), 12.94 KB, created by Ralph Böhme on 2023-06-14 16:56:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2023-06-14 16:56:25 UTC

Size: 12.94 KB

patch

obsolete

>From 6f4a6140e931222504bdd0bb449e5b7def21276e Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 6 Jun 2023 15:17:26 +0200
>Subject: [PATCH 1/2] CVE-2023-34968: mdssvc: cache and reuse stat info in
> struct sl_inode_path_map
>
>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 d679f89252707adc2d61ce782ac01cda5685a363 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Mon, 5 Jun 2023 18:02:20 +0200
>Subject: [PATCH 2/2] 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>
>---
> python/samba/tests/blackbox/mdsearch.py   |  8 +--
> python/samba/tests/dcerpc/mdssvc.py       | 26 +++++-----
> source3/rpc_client/cli_mdssvc.c           | 14 +++++-
> source3/rpc_server/mdssvc/mdssvc.c        | 60 +++++++++++++++++++++--
> source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 18 +++++--
> 5 files changed, 99 insertions(+), 27 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 046d37135cbc..01de378ce4d1 100644
>--- a/source3/rpc_client/cli_mdssvc.c
>+++ b/source3/rpc_client/cli_mdssvc.c
>@@ -725,6 +725,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;
> 
>@@ -759,7 +760,18 @@ 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 */
>+	p = strchr(path + 1, '/');
>+	if (p == NULL) {
>+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
>+		return;
>+	}
>+	state->path = talloc_strdup(state, p + 1);
>+	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_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
>index a6d09a43b9c8..aa4747d98acd 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;
>@@ -824,6 +850,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
>  **/
>@@ -936,8 +988,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 2fca15cb8a8d..b4970d75033f 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