From 94fcbec8af5cd877e43ca125912cff0d867f7215 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 17 Jun 2023 13:53:27 +0200 Subject: [PATCH] CVE-2023-34968: mdscli: return share relative paths MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 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 Reviewed-by: Stefan Metzmacher --- 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.11.4.GIT