From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 22 Oct 2021 02:27:25 +0000 (+1300)
Subject: CVE-2020-25722 s4/dsdb/samldb: check sAMAccountName for illegal characters
X-Git-Tag: ldb-2.5.0~212
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=510378f94a62313777da09efebf4bf737b23cd55;p=thirdparty%2Fsamba.git

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
index e53d566816e..00000000000
--- 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 af32e942c82..b0628df1131 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -325,6 +325,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);
@@ -424,6 +477,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(