lib: security: se_access_check() incorrectly processes owner rights (S-1-3-4) DENY...

Reported and proposed fix by Shilpa K <shilpa.krishnareddy@gmail.com>.

When processing DENY ACE entries for owner rights SIDs (S-1-3-4) the
code OR's in the deny access mask bits without taking into account if
they were being requested in the requested access mask.

E.g. The current logic has:

An ACL containining:

[0] SID: S-1-3-4
    TYPE: DENY
    MASK: WRITE_DATA
[1] SID: S-1-3-4
    TYPE: ALLOW
    MASK: ALLOW_ALL

prohibits an open request by the owner for READ_DATA - even though this
is explicitly allowed.

Furthermore a non-canonical ACL containing:

[0] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: READ_DATA

[1] SID: S-1-3-4
    TYPE: DENY
    MASK: READ_DATA

[2] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: WRITE_DATA

prohibits an open request by the owner for READ_DATA|WRITE_DATA - even
though READ_DATA is explicitly allowed in ACE no 0 and is thus already
filtered out of the "access-still-needed" mask when the deny ACE no 1 is
evaluated.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12466

Signed-off-by: Jeremy Allison <jra@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index 2be59289347a7bf126bb00c4371bfef187729891..b4c850b613e120b1e79fff85196342ac387bf083 100644 (file)
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -220,7 +220,7 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
                                owner_rights_allowed |= ace->access_mask;
                                owner_rights_default = false;
                        } else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED) {
-                               owner_rights_denied |= ace->access_mask;
+                               owner_rights_denied |= (bits_remaining & ace->access_mask);
                                owner_rights_default = false;
                        }
                        continue;