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>

diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c
index 892a844e71a870b28e6ed20a25603702ae457718..a39202d0c99ed2bfc8b59e24476a81b3f1be2687 100644 (file)
--- 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 d794ba158389d451201e885d243f9ad765004bbf..5f866d7fb6ea6c4277bb2c476c3959b4e1d412ae 100644 (file)
--- 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 086ca740604165bd00a36fe93f9e41247ff08ade..2cc1b44712cb1b317e34030da6f039ca6d2f0fc6 100644 (file)
--- 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 070503f9eeb819bcd0c1d6f90979dd2fb0f4306e..9b814f51fc36a10165519057554ebd52b13fed39 100644 (file)
--- a/source3/rpc_server/mdssvc/mdssvc.c
+++ b/source3/rpc_server/mdssvc/mdssvc.c
@@ -1732,11 +1732,11 @@ error:
  **/
 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;
@@ -1744,6 +1744,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;
@@ -1810,15 +1811,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 ff36b329f2b0c59dc82216ac286b903756c2dd44..e1fc0709ab17c582ec35e0251c3537bb39fc2b88 100644 (file)
--- 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 2fec2bb67251b14c64a6ad7be125bfccaf9f2cb9..ae6a73605e190f0328bcee424925043b7ff71cdc 100644 (file)
--- 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 64fe321089c41f933a07f47d18f50afbb9858d2f..ba3f61fd4b0603f167b605d348d446a3bcffd8fb 100644 (file)
--- 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 a16bd5b47e3a76b083b5f7779fc1f492c51c48bb..afe7068ee1bbad8692f58edb6d093836c0edb2e4 100644 (file)
--- 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,