From 2c061b96d3c52503fc436b6f0586c462f13d999f Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Sat, 25 Apr 2020 22:15:47 +0300
Subject: [PATCH] kdb: fix memory handling in ipadb_find_principal

BER structure representing a string might not have termination '\0'
character, thus we should use length-bound functions to operate on it.

Memory handling of LDAP values was leaving previous vals over iteration.
Also, when freeing vals, we need to explicitly set it to NULL.

Related: https://pagure.io/freeipa/issues/8291
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index da0b841a1..e583071da 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -1057,7 +1057,7 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext,
         for (int i = 0; vals[i]; i++) {
 #ifdef KRB5_KDB_FLAG_ALIAS_OK
             if ((flags & KRB5_KDB_FLAG_ALIAS_OK) == 0) {
-                found = strcmp(vals[i]->bv_val, *principal) == 0;
+                found = strncmp(vals[i]->bv_val, *principal, vals[i]->bv_len) == 0;
                 if (found)
                     break;
 
@@ -1080,7 +1080,7 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext,
              * name/alias is returned even if krbCanonicalName is not
              * present. */
             free(*principal);
-            *principal = strdup(vals[i]->bv_val);
+            *principal = strndup(vals[i]->bv_val, vals[i]->bv_len);
             if (!*principal) {
                 ret = KRB5_KDB_INTERNAL_ERROR;
                 goto done;
@@ -1088,11 +1088,14 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext,
             found = true;
             break;
         }
-        if (!found)
+
+        ldap_value_free_len(vals);
+	vals = NULL;
+        if (!found) {
             continue;
+        }
 
         /* We need to check if this is the canonical name. */
-        ldap_value_free_len(vals);
         vals = ldap_get_values_len(ipactx->lcontext, le, "krbcanonicalname");
         if (vals == NULL)
             break;
@@ -1101,16 +1104,17 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext,
         /* If aliases aren't accepted by the KDC, use case-sensitive
          * comparison. */
         if ((flags & KRB5_KDB_FLAG_ALIAS_OK) == 0) {
-            found = strcmp(vals[0]->bv_val, *principal) == 0;
+            found = strncmp(vals[0]->bv_val, *principal, vals[0]->bv_len) == 0;
             if (!found) {
                 ldap_value_free_len(vals);
+		vals = NULL;
                 continue;
             }
         }
 #endif
 
         free(*principal);
-        *principal = strdup(vals[0]->bv_val);
+        *principal = strndup(vals[0]->bv_val, vals[0]->bv_len);
         if (!*principal) {
             ret = KRB5_KDB_INTERNAL_ERROR;
             goto done;
-- 
2.25.3