From 1648576fb9d5dfcfdcb9b2a0cb830e214285e208 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Nov 09 2020 14:40:29 +0000
Subject: 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>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>

---

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 692f542..f2bd60e 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2611,7 +2611,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 7f1ae7f..368a2f9 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 BLACKLIST_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_blacklist_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);