From 81cbee4e3ff2e667946e0d41097b402257608b7e Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@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 <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_mspac.c       |  2 +-
 daemons/ipa-kdb/tests/ipa_kdb_tests.c | 50 +++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index dd29db190..fe5b586b6 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 d3ef5c00d..752b24ea4 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -71,6 +71,10 @@
 #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 @@
     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 @@
                         &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_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 @@
         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);
-- 
2.29.2