The Samba-Bugzilla – Attachment 17952 Details for
Bug 15388
[SECURITY] CVE-2023-34968: Spotlight server-side Share Path Disclosure
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.16
CVE-2023-34968-server_path-4.16-01.patch (text/plain), 54.43 KB, created by
Ralph Böhme
on 2023-06-23 16:48:09 UTC
(
hide
)
Description:
Patch for 4.16
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2023-06-23 16:48:09 UTC
Size:
54.43 KB
patch
obsolete
>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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
metze
:
review+
slow
:
ci-passed+
Actions:
View
Attachments on
bug 15388
:
17912
|
17918
|
17921
|
17932
|
17933
|
17936
|
17937
|
17945
|
17946
| 17952