From ddc88fa8b6e8c7facfc4b972d8316989b11eeade Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 29 Nov 2024 13:06:03 +0100 Subject: [PATCH] libcli: Make handling implicit_owner_rights bit easier to read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first time I came across this I missed the "FALL_THROUGH" and had to look closely at what happens. I had expected IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS to grant two rights, which to me is now more obvious. It was correct before, but to me this is now more obvious. YMMV. Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský --- libcli/security/access_check.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 3dc982332da..56050ca84c7 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -245,8 +245,9 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd, if (security_token_has_sid(token, sd->owner_sid)) { switch (implicit_owner_rights) { case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS: - granted |= SEC_STD_WRITE_DAC; - FALL_THROUGH; + granted |= (SEC_STD_READ_CONTROL | + SEC_STD_WRITE_DAC); + break; case IMPLICIT_OWNER_READ_CONTROL_RIGHTS: granted |= SEC_STD_READ_CONTROL; break; @@ -282,8 +283,8 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd, if (am_owner && !have_owner_rights_ace) { switch (implicit_owner_rights) { case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS: - granted |= SEC_STD_WRITE_DAC; - FALL_THROUGH; + granted |= (SEC_STD_READ_CONTROL | SEC_STD_WRITE_DAC); + break; case IMPLICIT_OWNER_READ_CONTROL_RIGHTS: granted |= SEC_STD_READ_CONTROL; break; @@ -436,8 +437,9 @@ static NTSTATUS se_access_check_implicit_owner(const struct security_descriptor if (am_owner && !have_owner_rights_ace) { switch (implicit_owner_rights) { case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS: - bits_remaining &= ~SEC_STD_WRITE_DAC; - FALL_THROUGH; + bits_remaining &= ~(SEC_STD_WRITE_DAC | + SEC_STD_READ_CONTROL); + break; case IMPLICIT_OWNER_READ_CONTROL_RIGHTS: bits_remaining &= ~SEC_STD_READ_CONTROL; break; @@ -751,8 +753,9 @@ NTSTATUS sec_access_check_ds_implicit_owner(const struct security_descriptor *sd security_token_has_sid(token, sd->owner_sid)) { switch (implicit_owner_rights) { case IMPLICIT_OWNER_READ_CONTROL_AND_WRITE_DAC_RIGHTS: - bits_remaining &= ~SEC_STD_WRITE_DAC; - FALL_THROUGH; + bits_remaining &= ~(SEC_STD_WRITE_DAC | + SEC_STD_READ_CONTROL); + break; case IMPLICIT_OWNER_READ_CONTROL_RIGHTS: bits_remaining &= ~SEC_STD_READ_CONTROL; break;