diff options
author | Joseph Sutton <josephsutton@catalyst.net.nz> | 2022-05-25 20:00:55 +1200 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2022-07-24 11:42:02 +0200 |
commit | d40593be83144713cfc43e4eb1c7bc2d925a0da0 (patch) | |
tree | f455d3850c607703130ab8ae333bb70052d1304f | |
parent | 389851bcf399f9511e2cb797350c37ce91aa5849 (diff) | |
download | samba-d40593be83144713cfc43e4eb1c7bc2d925a0da0.tar.gz |
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>
-rw-r--r-- | selftest/knownfail_heimdal_kdc | 4 | ||||
-rw-r--r-- | selftest/knownfail_mit_kdc | 4 | ||||
-rw-r--r-- | source4/kdc/db-glue.c | 27 |
3 files changed, 22 insertions, 13 deletions
diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index dbfff5784e6..afb9bcf1209 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -278,7 +278,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 0f90ea10299..c2a31b4a140 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -583,7 +583,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 073ec83c8cf..cfa2097acbd 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -769,15 +769,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; @@ -789,13 +793,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, |