From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Wed, 25 May 2022 08:00:55 +0000 (+1200)
Subject: CVE-2022-2031 s4:kdc: Don't use strncmp to compare principal components
X-Git-Tag: samba-4.17.0rc1~277
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=09e54a7b1d18f2fdb3ebe47dadcea12c52bd8810;p=thirdparty%2Fsamba.git

CVE-2022-2031 s4:kdc: Don't use strncmp to compare principal components

We would only compare the first 'n' characters, where 'n' is the length
of the principal component string, so 'k@REALM' would erroneously be
considered equal to 'krbtgt@REALM'.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
---

diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc
index 403735d09b9..341cdfb21c1 100644
--- a/selftest/knownfail_heimdal_kdc
+++ b/selftest/knownfail_heimdal_kdc
@@ -60,7 +60,3 @@
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc
-#
-# AS-REQ tests
-#
-^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\(
diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc
index d1413f10692..e1315ed71ac 100644
--- a/selftest/knownfail_mit_kdc
+++ b/selftest/knownfail_mit_kdc
@@ -441,7 +441,3 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_service.ad_dc
-#
-# AS-REQ tests
-#
-^samba.tests.krb5.as_req_tests.samba.tests.krb5.as_req_tests.AsReqKerberosTests.test_krbtgt_wrong_principal\(
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 68eab93c680..024073eb5bd 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -870,15 +870,19 @@ static int principal_comp_strcmp_int(krb5_context context,
 				     bool do_strcasecmp)
 {
 	const char *p;
-	size_t len;
 
 #if defined(HAVE_KRB5_PRINCIPAL_GET_COMP_STRING)
 	p = krb5_principal_get_comp_string(context, principal, component);
 	if (p == NULL) {
 		return -1;
 	}
-	len = strlen(p);
+	if (do_strcasecmp) {
+		return strcasecmp(p, string);
+	} else {
+		return strcmp(p, string);
+	}
 #else
+	size_t len;
 	krb5_data *d;
 	if (component >= krb5_princ_size(context, principal)) {
 		return -1;
@@ -890,13 +894,26 @@ static int principal_comp_strcmp_int(krb5_context context,
 	}
 
 	p = d->data;
-	len = d->length;
-#endif
+
+	len = strlen(string);
+
+	/*
+	 * We explicitly return -1 or 1. Subtracting of the two lengths might
+	 * give the wrong result if the result overflows or loses data when
+	 * narrowed to int.
+	 */
+	if (d->length < len) {
+		return -1;
+	} else if (d->length > len) {
+		return 1;
+	}
+
 	if (do_strcasecmp) {
 		return strncasecmp(p, string, len);
 	} else {
-		return strncmp(p, string, len);
+		return memcmp(p, string, len);
 	}
+#endif
 }
 
 static int principal_comp_strcasecmp(krb5_context context,