From 9be11622765c060971c4fcc2fba981f760f897d8 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 22 Oct 2021 15:27:25 +1300
Subject: [PATCH 154/266] 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>

Conflict:remove test
Reference:https://gitlab.com/samba-team/samba/-/commit/9be11622765c060971c4fcc2fba981f760f897d8

---
 source4/dsdb/samdb/ldb_modules/samldb.c   | 58 +++++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 0cf00e2b19e..f420009376c 100644
--- 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(
-- 
2.23.0

