CVE-2020-25722 s4/dsdb/samldb: check sAMAccountName for illegal characters

This only for the real account name, not the account name implicit in
a UPN. It doesn't matter if a UPN implies an illegal sAMAccountName,
since that is not going to conflict with a real one.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/selftest/knownfail.d/ldap_upn_sam_account b/selftest/knownfail.d/ldap_upn_sam_account
deleted file mode 100644 (file)
index e53d566..0000000
--- a/selftest/knownfail.d/ldap_upn_sam_account
+++ /dev/null
@@ -1 +0,0 @@
-samba.tests.ldap_upn_sam_account.+LdapUpnSamSambaOnlyTest.test_upn_sam_SAM_contains_delete

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 0cf00e2b19e564a7559036a9f42f67cf1423989e..f420009376ccaae67de2894dc6f035c1a63ae13a 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -322,6 +322,59 @@ static inline int samldb_sam_account_upn_clash_sub_search(
        return LDB_SUCCESS;
 }
 
+static int samaccountname_bad_chars_check(struct samldb_ctx *ac,
+                                         const char *name)
+{
+       /*
+        * The rules here are based on
+        *
+        * https://social.technet.microsoft.com/wiki/contents/articles/11216.active-directory-requirements-for-creating-objects.aspx
+        *
+        * Windows considers UTF-8 sequences that map to "similar" characters
+        * (e.g. 'a', 'ā') to be the same sAMAccountName, and we don't. Names
+        * that are not valid UTF-8 *are* allowed.
+        *
+        * Additionally, Samba collapses multiple spaces, and Windows doesn't.
+        */
+       struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+       size_t i;
+
+       for (i = 0; name[i] != '\0'; i++) {
+               uint8_t c = name[i];
+               char *p = NULL;
+               if (c < 32 || c == 127) {
+                       ldb_asprintf_errstring(
+                               ldb,
+                               "samldb: sAMAccountName contains invalid "
+                               "0x%.2x character\n", c);
+                       return LDB_ERR_CONSTRAINT_VIOLATION;
+               }
+               p = strchr("\"[]:;|=+*?<>/\\,", c);
+               if (p != NULL) {
+                       ldb_asprintf_errstring(
+                               ldb,
+                               "samldb: sAMAccountName contains invalid "
+                               "'%c' character\n", c);
+                       return LDB_ERR_CONSTRAINT_VIOLATION;
+               }
+       }
+
+       if (i == 0) {
+               ldb_asprintf_errstring(
+                       ldb,
+                       "samldb: sAMAccountName is empty\n");
+               return LDB_ERR_CONSTRAINT_VIOLATION;
+       }
+
+       if (name[i - 1] == '.') {
+               ldb_asprintf_errstring(
+                       ldb,
+                       "samldb: sAMAccountName ends with '.'");
+               return LDB_ERR_CONSTRAINT_VIOLATION;
+       }
+       return LDB_SUCCESS;
+}
+
 static int samldb_sam_account_upn_clash(struct samldb_ctx *ac)
 {
        struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
@@ -421,6 +474,11 @@ static int samldb_sam_account_upn_clash(struct samldb_ctx *ac)
                        talloc_free(tmp_ctx);
                        return ret;
                }
+               ret = samaccountname_bad_chars_check(ac, real_sam);
+               if (ret != LDB_SUCCESS) {
+                       talloc_free(tmp_ctx);
+                       return ret;
+               }
        }
        if (implied_upn != NULL) {
                ret = samldb_sam_account_upn_clash_sub_search(