CVE-2023-0614 s4-acl: Split out logic to remove access checking attributes

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 7585be3f93b2f0fa5f25be4e81a5d006d2bb0e50..884ba268cca6571164edefd7631537632de6fbed 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -546,6 +546,39 @@ static int check_search_ops_access(struct aclread_context *ac,
        return ret;
 }
 
+/*
+ * Whether this attribute was added to perform access checks and must be
+ * removed.
+ */
+static bool should_remove_attr(const char *attr, const struct aclread_context *ac)
+{
+       if (ac->added_nTSecurityDescriptor &&
+           ldb_attr_cmp("nTSecurityDescriptor", attr) == 0)
+       {
+               return true;
+       }
+
+       if (ac->added_objectSid &&
+           ldb_attr_cmp("objectSid", attr) == 0)
+       {
+               return true;
+       }
+
+       if (ac->added_instanceType &&
+           ldb_attr_cmp("instanceType", attr) == 0)
+       {
+               return true;
+       }
+
+       if (ac->added_objectClass &&
+           ldb_attr_cmp("objectClass", attr) == 0)
+       {
+               return true;
+       }
+
+       return false;
+}
+
 static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
 {
        struct ldb_context *ldb;
@@ -620,7 +653,6 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
                /* for every element in the message check RP */
                for (i=0; i < msg->num_elements; i++) {
                        const struct dsdb_attribute *attr;
-                       bool is_sd, is_objectsid, is_instancetype, is_objectclass;
                        uint32_t access_mask;
                        attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
                                                                 msg->elements[i].name);
@@ -632,28 +664,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
                                ret = LDB_ERR_OPERATIONS_ERROR;
                                goto fail;
                        }
-                       is_sd = ldb_attr_cmp("nTSecurityDescriptor",
-                                             msg->elements[i].name) == 0;
-                       is_objectsid = ldb_attr_cmp("objectSid",
-                                                   msg->elements[i].name) == 0;
-                       is_instancetype = ldb_attr_cmp("instanceType",
-                                                      msg->elements[i].name) == 0;
-                       is_objectclass = ldb_attr_cmp("objectClass",
-                                                     msg->elements[i].name) == 0;
-                       /* these attributes were added to perform access checks and must be removed */
-                       if (is_objectsid && ac->added_objectSid) {
-                               ldb_msg_element_mark_inaccessible(&msg->elements[i]);
-                               continue;
-                       }
-                       if (is_instancetype && ac->added_instanceType) {
-                               ldb_msg_element_mark_inaccessible(&msg->elements[i]);
-                               continue;
-                       }
-                       if (is_objectclass && ac->added_objectClass) {
-                               ldb_msg_element_mark_inaccessible(&msg->elements[i]);
-                               continue;
-                       }
-                       if (is_sd && ac->added_nTSecurityDescriptor) {
+                       /* Remove attributes added to perform access checks. */
+                       if (should_remove_attr(msg->elements[i].name, ac)) {
                                ldb_msg_element_mark_inaccessible(&msg->elements[i]);
                                continue;
                        }