CVE-2023-34968: mdscli: return share relative paths
authorRalph Boehme <slow@samba.org>
Sat, 17 Jun 2023 11:53:27 +0000 (13:53 +0200)
committerJule Anger <janger@samba.org>
Fri, 14 Jul 2023 13:15:01 +0000 (15:15 +0200)
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
python/samba/tests/dcerpc/mdssvc.py
source3/rpc_client/cli_mdssvc.c
source3/rpc_client/cli_mdssvc_private.h
source3/rpc_client/cli_mdssvc_util.c
source3/rpc_client/cli_mdssvc_util.h

index c9156ae6e0e414927d883ed7174815b2d0f04442..c8e75661f151ac0c0eba1fb7663abbc291fad9f7 100644 (file)
@@ -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)
index b0df509ddc793c3eadf1f6bd1c5d3d7876269b24..5002e5d26d64926496792258c557cdd82f348a84 100644 (file)
@@ -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)
index 474d7c0b15004f46178249c19c00a0a21873ebce..753bc2e52edff498504c247f764d3ff5316bb843 100644 (file)
@@ -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);
index 031af85bf583cf623a809682ba4cb4a2af89e92f..77f300c09ccc891901d0b071d770c896c1d9ab91 100644 (file)
@@ -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 {
index a39202d0c99ed2bfc8b59e24476a81b3f1be2687..1eaaca715a80fce82c8fe42751da56fcabd3214b 100644 (file)
 #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)
index 7a98c85452674059a4894186c678a7b6fdea7c46..3f324758c70730a9053252cb5b56c03f65c30d34 100644 (file)
 #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);