From: Ralph Boehme Date: Mon, 19 Jun 2023 16:16:57 +0000 (+0200) Subject: CVE-2023-34968: mdssvc: introduce an allocating wrapper to sl_pack() X-Git-Tag: samba-4.16.11~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d6b9c5234ffc6bf415156c693aac3256d17e259c;p=thirdparty%2Fsamba.git 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 Reviewed-by: Stefan Metzmacher --- diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c index 892a844e71a..a39202d0c99 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 441d41160f1..34bfda5eca6 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 086ca740604..2cc1b44712c 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 92afa9e5fd4..0e62cba7038 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -1745,11 +1745,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; @@ -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 ff36b329f2b..e1fc0709ab1 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 2fec2bb6725..ae6a73605e1 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 64fe321089c..ba3f61fd4b0 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 a16bd5b47e3..afe7068ee1b 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,