From f2d473d1b6a2bb243fcf2ae933807ea58681d655 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 23 Mar 2024 08:27:41 +1300 Subject: [PATCH] ndr: always attempt ACE coda pull if ACE type suggests a coda We were skipping the pull in cases where the coda size was calculated to be zero. This has the right result for empty conditional ACEs, but not for Resource Attribute ACEs where the CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 coda was not intialised. The situation is made a bit worse, because the function that calculates the coda size (ndr_subcontext_size_of_ace_coda()) can return zero in conditions that are not exactly errors, but in which the would-be calculated value makes so little sense that zero is thought to be a safer default. Credit to OSS-Fuzz. REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66577 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15613 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Mon Mar 25 06:00:21 UTC 2024 on atb-devel-224 (cherry picked from commit 6fb98f70c6274e172787c8d5f73aa93920171e7c) --- librpc/ndr/ndr_sec_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/librpc/ndr/ndr_sec_helper.c b/librpc/ndr/ndr_sec_helper.c index f870a17aafc..1a156b01d40 100644 --- a/librpc/ndr/ndr_sec_helper.c +++ b/librpc/ndr/ndr_sec_helper.c @@ -104,7 +104,6 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags { NDR_PULL_CHECK_FLAGS(ndr, ndr_flags); if (ndr_flags & NDR_SCALARS) { - ssize_t sub_size; NDR_CHECK(ndr_pull_align(ndr, 5)); NDR_CHECK(ndr_pull_security_ace_type(ndr, NDR_SCALARS, &r->type)); NDR_CHECK(ndr_pull_security_ace_flags(ndr, NDR_SCALARS, &r->flags)); @@ -112,12 +111,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_security_ace(struct ndr_pull *ndr, ndr_flags NDR_CHECK(ndr_pull_uint32(ndr, NDR_SCALARS, &r->access_mask)); NDR_CHECK(ndr_maybe_pull_security_ace_object_ctr(ndr, NDR_SCALARS, r)); NDR_CHECK(ndr_pull_dom_sid(ndr, NDR_SCALARS, &r->trustee)); - sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags); - if (!sec_ace_has_extra_blob(r->type) || sub_size == 0) { + if (!sec_ace_has_extra_blob(r->type)) { r->coda.ignored.data = NULL; r->coda.ignored.length = 0; } else { struct ndr_pull *_ndr_coda; + ssize_t sub_size = ndr_subcontext_size_of_ace_coda(r, r->size, ndr->flags); NDR_CHECK(ndr_pull_subcontext_start(ndr, &_ndr_coda, 0, sub_size)); NDR_CHECK(ndr_pull_set_switch_value(_ndr_coda, &r->coda, r->type)); NDR_CHECK(ndr_pull_security_ace_coda(_ndr_coda, NDR_SCALARS|NDR_BUFFERS, &r->coda)); -- 2.25.1