CVE-2018-10919 security: Fix checking of object-specific CONTROL_ACCESS rights

An 'Object Access Allowed' ACE that assigned 'Control Access' (CR)
rights to a specific attribute would not actually grant access.

What was happening was the remaining_access mask for the object_tree
nodes would be Read Property (RP) + Control Access (CR). The ACE mapped
to the schemaIDGUID for a given attribute, which would end up being a
child node in the tree. So the CR bit was cleared for a child node, but
not the rest of the tree. We would then check the user had the RP access
right, which it did. However, the RP right was cleared for another node
in the tree, which still had the CR bit set in its remaining_access
bitmap, so Samba would not grant access.

Generally, the remaining_access only ever has one bit set, which means
this isn't a problem normally. However, in the Control Access case there
are 2 separate bits being checked, i.e. RP + CR.

One option to fix this problem would be to clear the remaining_access
for the tree instead of just the node. However, the Windows spec is
actually pretty clear on this: if the ACE has a CR right present, then
you can stop any further access checks.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index 93eb85def919cf9712c455686f1f0561a41fe241..03a7dca4adf899d9e72c1d0c52645db32aa17989 100644 (file)
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -429,6 +429,16 @@ static NTSTATUS check_object_specific_access(struct security_ace *ace,
                        *grant_access = true;
                        return NT_STATUS_OK;
                }
+
+               /*
+                * As per 5.1.3.3.4 Checking Control Access Right-Based Access,
+                * if the CONTROL_ACCESS right is present, then we can grant
+                * access and stop any further access checks
+                */
+               if (ace->access_mask & SEC_ADS_CONTROL_ACCESS) {
+                       *grant_access = true;
+                       return NT_STATUS_OK;
+               }
        } else {
 
                /* this ACE denies access to the requested object/attribute */