CVE-2022-2031 s4:kdc: Don't use strncmp to compare principal components
authorJoseph Sutton <josephsutton@catalyst.net.nz>
Wed, 25 May 2022 08:00:55 +0000 (20:00 +1200)
committerJule Anger <janger@samba.org>
Sun, 24 Jul 2022 07:23:56 +0000 (09:23 +0200)
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>
selftest/knownfail_heimdal_kdc
selftest/knownfail_mit_kdc
source4/kdc/db-glue.c

index 26fc210c6f553898723e3106c62d10fd87923dcf..fb6ac228c5445da659d7efde920a8a2858baf022 100644 (file)
@@ -53,7 +53,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\(
index 8bf0b88550eb8e59f8af9c73d45815e150483e11..13dd806aeb10e2033bdb931856477e4f43aed58a 100644 (file)
@@ -552,7 +552,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\(
index a936bd1fa836c196995016f20c27928c4d603030..231b30b623f236c0f6c06c510dcd2c7a826cf2bb 100644 (file)
@@ -833,15 +833,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;
@@ -853,13 +857,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,