CVE-2023-0225 s4-acl: Don't return early if dNSHostName element has no values

This early return would mistakenly allow an unprivileged user to delete
the dNSHostName attribute by making an LDAP modify request with no
values. We should no longer allow this.

Add or replace operations with no values and no privileges are
disallowed.

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

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

diff --git a/selftest/knownfail.d/dns-host-name-deletion b/selftest/knownfail.d/dns-host-name-deletion
deleted file mode 100644 (file)
index ac11619..0000000
--- a/selftest/knownfail.d/dns-host-name-deletion
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_ldif_unspecified\(.*\)
-^samba4.ldap.acl_modify.python\(.*\).__main__.AclModifyTests.test_modify_delete_dns_host_name_unspecified\(.*\)

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 343cd8325fc3197f0df8a502f6a681f48143e553..f6fab910d62db534c3f572614b9a8075c18cdc96 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -844,11 +844,6 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
                NULL
        };
 
-       if (el->num_values == 0) {
-               return LDB_SUCCESS;
-       }
-       dnsHostName = &el->values[0];
-
        tmp_ctx = talloc_new(mem_ctx);
        if (tmp_ctx == NULL) {
                return ldb_oom(ldb);
@@ -999,6 +994,13 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
                --account_name_len;
        }
 
+       /* Check for add or replace requests with no value. */
+       if (el->num_values == 0) {
+               talloc_free(tmp_ctx);
+               return ldb_operr(ldb);
+       }
+       dnsHostName = &el->values[0];
+
        dnsHostName_str = (const char *)dnsHostName->data;
        dns_host_name_len = dnsHostName->length;