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 <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
This commit is contained in:
Alexander Bokovoy 2020-11-06 14:07:10 +02:00 committed by Rob Crittenden
parent 69b42f0c80
commit 81cbee4e3f
2 changed files with 51 additions and 1 deletions

View File

@ -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;

View File

@ -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,38 @@ void test_dom_sid_string(void **state)
}
void test_check_trusted_realms(void **state)
{
struct test_ctx *test_ctx;
krb5_error_code kerr = 0;
char *trusted_realm = NULL;
test_ctx = (struct test_ctx *) *state;
for(size_t i = 0; i < NUM_SUFFIXES; i++) {
char *test_realm = NULL;
asprintf(&test_realm, TEST_REALM_TEMPLATE, i);
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);
}
}
kerr = ipadb_is_princ_from_trusted_realm(
test_ctx->krb5_ctx,
EXTERNAL_REALM,
strlen(EXTERNAL_REALM),
&trusted_realm);
assert_int_equal(kerr, KRB5_KDB_NOENTRY);
}
int main(int argc, const char *argv[])
{
const struct CMUnitTest tests[] = {
@ -488,6 +536,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);