URL: https://github.com/freeipa/freeipa/pull/5232 Author: abbra Title: #5232: ipa-kdb: fix crash in MS-PAC cache init code Action: opened
PR body: """ When initializing UPN suffixes, we calculate their sizes and didn't use the right variable to allocate their size. This affects us if there are more than one UPN suffix available for a trust due to memory corruption while filling in sizes. Add unit test for multiple UPN suffixes. Fixes: https://pagure.io/freeipa/issue/8566 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5232/head:pr5232 git checkout pr5232
From 2f0e1173c5b4acc616bd565597335dab7209e2b2 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Fri, 6 Nov 2020 14:07:10 +0200 Subject: [PATCH] ipa-kdb: fix crash in MS-PAC cache init code When initializing UPN suffixes, we calculate their sizes and didn't use the right variable to allocate their size. This affects us if there are more than one UPN suffix available for a trust due to memory corruption while filling in sizes. Add unit test for multiple UPN suffixes. Fixes: https://pagure.io/freeipa/issue/8566 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_mspac.c | 2 +- daemons/ipa-kdb/tests/ipa_kdb_tests.c | 56 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index dd29db19010..fe5b586b6ae 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -2610,7 +2610,7 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) for (; t[n].upn_suffixes[len] != NULL; len++); if (len != 0) { - t[n].upn_suffixes_len = calloc(n, sizeof(size_t)); + t[n].upn_suffixes_len = calloc(len, sizeof(size_t)); if (t[n].upn_suffixes_len == NULL) { ret = ENOMEM; goto done; diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c index d3ef5c00d47..644660e9fba 100644 --- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c +++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c @@ -71,6 +71,10 @@ struct test_ctx { #define DOM_SID "S-1-5-21-1-2-3" #define DOM_SID_TRUST "S-1-5-21-4-5-6" #define BLOCKLIST_SID "S-1-5-1" +#define NUM_SUFFIXES 10 +#define SUFFIX_TEMPLATE "d%0d" DOMAIN_NAME +#define TEST_REALM_TEMPLATE "some." SUFFIX_TEMPLATE +#define EXTERNAL_REALM "WRONG.DOMAIN" static int setup(void **state) { @@ -92,6 +96,9 @@ static int setup(void **state) ipa_ctx = calloc(1, sizeof(struct ipadb_context)); assert_non_null(ipa_ctx); + kerr = krb5_get_default_realm(krb5_ctx, &ipa_ctx->realm); + assert_int_equal(kerr, 0); + ipa_ctx->mspac = calloc(1, sizeof(struct ipadb_mspac)); assert_non_null(ipa_ctx->mspac); @@ -126,6 +133,15 @@ static int setup(void **state) &ipa_ctx->mspac->trusts[0].sid_blocklist_incoming[0]); assert_int_equal(ret, 0); + ipa_ctx->mspac->trusts[0].upn_suffixes = calloc(NUM_SUFFIXES + 1, sizeof(char *)); + ipa_ctx->mspac->trusts[0].upn_suffixes_len = calloc(NUM_SUFFIXES, sizeof(size_t)); + for (size_t i = 0; i < NUM_SUFFIXES; i++) { + asprintf(&(ipa_ctx->mspac->trusts[0].upn_suffixes[i]), SUFFIX_TEMPLATE, i); + ipa_ctx->mspac->trusts[0].upn_suffixes_len[i] = + strlen(ipa_ctx->mspac->trusts[0].upn_suffixes[i]); + + } + ipa_ctx->kcontext = krb5_ctx; kerr = krb5_db_set_context(krb5_ctx, ipa_ctx); assert_int_equal(kerr, 0); @@ -478,6 +494,44 @@ void test_dom_sid_string(void **state) } +void test_check_trusted_realms(void **state) +{ + struct test_ctx *test_ctx; + + test_ctx = (struct test_ctx *) *state; + + for(size_t i = 0; i < NUM_SUFFIXES; i++) { + char *test_realm = NULL; + char *trusted_realm = NULL; + asprintf(&test_realm, TEST_REALM_TEMPLATE, i); + krb5_error_code kerr = 0; + + if (test_realm) { + kerr = ipadb_is_princ_from_trusted_realm( + test_ctx->krb5_ctx, + test_realm, + strlen(test_realm), + &trusted_realm); + assert_int_equal(kerr, 0); + free(test_realm); + free(trusted_realm); + } + + asprintf(&test_realm, EXTERNAL_REALM, i); + kerr = 0; + + if (test_realm) { + kerr = ipadb_is_princ_from_trusted_realm( + test_ctx->krb5_ctx, + test_realm, + strlen(test_realm), + &trusted_realm); + assert_int_equal(kerr, KRB5_KDB_NOENTRY); + free(test_realm); + } + } +} + int main(int argc, const char *argv[]) { const struct CMUnitTest tests[] = { @@ -488,6 +542,8 @@ int main(int argc, const char *argv[]) cmocka_unit_test(test_string_to_sid), cmocka_unit_test_setup_teardown(test_dom_sid_string, setup, teardown), + cmocka_unit_test_setup_teardown(test_check_trusted_realms, + setup, teardown), }; return cmocka_run_group_tests(tests, NULL, NULL);
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
