Fix multiple libkdb_ldap memory leaks
authorGreg Hudson <ghudson@mit.edu>
Thu, 23 Jun 2011 19:25:51 +0000 (19:25 +0000)
committerGreg Hudson <ghudson@mit.edu>
Thu, 23 Jun 2011 19:25:51 +0000 (19:25 +0000)
* krb5_ldap_policydn_to_name wasn't freeing rdn, and was using the
  wrong function to free dn, in the HAVE_LDAP_STR2DN CASE.
* populate_krb5_db_entry wasn't freeing the tl_data generated from
  ber_tl_data.
* populate_krb5_db_entry was using the wrong function to free
  a password policy when finding pw_max_life.
* krb5_ldap_put_principal wasn't freeing ber_tl_data.
* krb5_update_tl_kadm_data had a bad contract.  Change the contract
  to be more like krb5_dbe_update_mod_princ_data and simplify its
  memory management.

ticket: 6924

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@24984 dc483132-0cff-0310-8789-dd5450dbe970

src/plugins/kdb/ldap/libkdb_ldap/ldap_misc.c
src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
src/plugins/kdb/ldap/libkdb_ldap/princ_xdr.c
src/plugins/kdb/ldap/libkdb_ldap/princ_xdr.h

index 14dec3a60ecaf695e6bc6107c354b34f2be0f584..4500bb69c78b8f79253d099f5717f6fdc4aa978a 100644 (file)
@@ -1663,7 +1663,9 @@ krb5_ldap_policydn_to_name(krb5_context context, char *policy_dn, char **name)
         LDAPDN dn;
         rdn = strndup(policy_dn, len2 - len1 - 1); /* 1 character for ',' */
 
-        if (ldap_str2dn (rdn, &dn, LDAP_DN_FORMAT_LDAPV3 | LDAP_DN_PEDANTIC) != 0) {
+        st = ldap_str2dn(rdn, &dn, LDAP_DN_FORMAT_LDAPV3 | LDAP_DN_PEDANTIC);
+        free(rdn);
+        if (st != 0) {
             st = EINVAL;
             goto cleanup;
         }
@@ -1677,7 +1679,7 @@ krb5_ldap_policydn_to_name(krb5_context context, char *policy_dn, char **name)
                 st = EINVAL;
         }
 
-        ldap_memfree (dn);
+        ldap_dnfree(dn);
     }
 #elif defined HAVE_LDAP_EXPLODE_DN
     {
@@ -1954,18 +1956,14 @@ populate_krb5_db_entry(krb5_context context, krb5_ldap_context *ldap_context,
                                  &attr_present)) != 0)
         goto cleanup;
     if (attr_present == TRUE) {
-        krb5_tl_data  kadm_tl_data;
-
         mask |= KDB_PWD_POL_REF_ATTR;
 
         /* Ensure that the policy is inside the realm container */
         if ((st = krb5_ldap_policydn_to_name (context, pwdpolicydn, &polname)) != 0)
             goto cleanup;
 
-        if ((st = krb5_update_tl_kadm_data(polname, &kadm_tl_data)) != 0) {
+        if ((st = krb5_update_tl_kadm_data(context, entry, polname)) != 0)
             goto cleanup;
-        }
-        krb5_dbe_update_tl_data(context, entry, &kadm_tl_data);
     }
 
     /* KRBSECRETKEY */
@@ -2073,7 +2071,10 @@ populate_krb5_db_entry(krb5_context context, krb5_ldap_context *ldap_context,
             for (i = 0; ber_tl_data[i] != NULL; i++) {
                 if ((st = berval2tl_data (ber_tl_data[i] , &ptr)) != 0)
                     break;
-                if ((st = krb5_dbe_update_tl_data(context, entry, ptr)) != 0)
+                st = krb5_dbe_update_tl_data(context, entry, ptr);
+                free(ptr->tl_data_contents);
+                free(ptr);
+                if (st != 0)
                     break;
             }
             ldap_value_free_len (ber_tl_data);
@@ -2134,7 +2135,7 @@ populate_krb5_db_entry(krb5_context context, krb5_ldap_context *ldap_context,
         if ((st=krb5_ldap_get_password_policy(context, polname, &pwdpol)) != 0)
             goto cleanup;
         pw_max_life = pwdpol->pw_max_life;
-        free (pwdpol);
+        krb5_ldap_free_password_policy(context, pwdpol);
 
         if (pw_max_life > 0) {
             if ((st=krb5_dbe_lookup_last_pwd_change(context, entry, &last_pw_changed)) != 0)
index 8fe4fb2c3541310237e1fef1730b331199f7e2b2..0e78354fca78ccc07a1e108d3a370d6a07753019 100644 (file)
@@ -1102,18 +1102,18 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
                     break;
                 j++;
             }
-            if (st != 0) {
-                for (j = 0; ber_tl_data[j] != NULL; j++) {
-                    free (ber_tl_data[j]->bv_val);
-                    free (ber_tl_data[j]);
-                }
-                free (ber_tl_data);
-                goto cleanup;
+            if (st == 0) {
+                ber_tl_data[count] = NULL;
+                st=krb5_add_ber_mem_ldap_mod(&mods, "krbExtraData",
+                                             LDAP_MOD_REPLACE |
+                                             LDAP_MOD_BVALUES, ber_tl_data);
             }
-            ber_tl_data[count] = NULL;
-            if ((st=krb5_add_ber_mem_ldap_mod(&mods, "krbExtraData",
-                                              LDAP_MOD_REPLACE | LDAP_MOD_BVALUES,
-                                              ber_tl_data)) != 0)
+            for (j = 0; ber_tl_data[j] != NULL; j++) {
+                free(ber_tl_data[j]->bv_val);
+                free(ber_tl_data[j]);
+            }
+            free(ber_tl_data);
+            if (st != 0)
                 goto cleanup;
         }
         if ((st=krb5_dbe_lookup_last_admin_unlock(context, entry,
index 501d263b13ab0bc68fa0dd995513d2921111ca7b..760bfd31ab82cebb37f6b9512902dd63d95186c7 100644 (file)
@@ -200,33 +200,28 @@ krb5_lookup_tl_kadm_data(krb5_tl_data *tl_data, osa_princ_ent_rec *princ_entry)
 }
 
 krb5_error_code
-krb5_update_tl_kadm_data(policy_dn, new_tl_data)
-    char               * policy_dn;
-    krb5_tl_data        * new_tl_data;
+krb5_update_tl_kadm_data(krb5_context context, krb5_db_entry *entry,
+                        char *policy_dn)
 {
     XDR xdrs;
-    osa_princ_ent_t princ_entry;
+    osa_princ_ent_rec princ_entry;
+    krb5_tl_data tl_data;
+    krb5_error_code retval;
 
-    if ((princ_entry = (osa_princ_ent_t) malloc(sizeof(osa_princ_ent_rec))) == NULL)
-       return ENOMEM;
-
-    memset(princ_entry, 0, sizeof(osa_princ_ent_rec));
-    princ_entry->admin_history_kvno = 2;
-    princ_entry->aux_attributes = KADM5_POLICY;
-    princ_entry->policy = policy_dn;
+    memset(&princ_entry, 0, sizeof(osa_princ_ent_rec));
+    princ_entry.admin_history_kvno = 2;
+    princ_entry.aux_attributes = KADM5_POLICY;
+    princ_entry.policy = policy_dn;
 
     xdralloc_create(&xdrs, XDR_ENCODE);
-    if (! ldap_xdr_osa_princ_ent_rec(&xdrs, princ_entry)) {
+    if (! ldap_xdr_osa_princ_ent_rec(&xdrs, &princ_entry)) {
        xdr_destroy(&xdrs);
-       return(KADM5_XDR_FAILURE);
+       return KADM5_XDR_FAILURE;
     }
-    new_tl_data->tl_data_type = KRB5_TL_KADM_DATA;
-    new_tl_data->tl_data_length = xdr_getpos(&xdrs);
-    new_tl_data->tl_data_contents = (krb5_octet *)xdralloc_getdata(&xdrs);
-
-    /*
-      xdr_destroy(&xdrs);
-      ldap_osa_free_princ_ent(princ_entry);
-    */
-    return(0);
+    tl_data.tl_data_type = KRB5_TL_KADM_DATA;
+    tl_data.tl_data_length = xdr_getpos(&xdrs);
+    tl_data.tl_data_contents = (krb5_octet *)xdralloc_getdata(&xdrs);
+    retval = krb5_dbe_update_tl_data(context, entry, &tl_data);
+    xdr_destroy(&xdrs);
+    return retval;
 }
index 5f40e4330a97b2175974e8c1a4d4beab3a8edc4f..78ce2d09943844d26f0c683ea240ed1ddcf91b7e 100644 (file)
@@ -56,6 +56,7 @@ krb5_error_code
 krb5_lookup_tl_kadm_data(krb5_tl_data *tl_data, osa_princ_ent_rec *princ_entry);
 
 krb5_error_code
-krb5_update_tl_kadm_data(char *, krb5_tl_data *);
+krb5_update_tl_kadm_data(krb5_context context, krb5_db_entry *entry,
+                        char *policy_dn);
 
 #endif