[ldns-users] Memory leak in keys.c, ldns_key_new_frm_algorithm

[Willem Toorop]

Hi Michael,

Indeed. I have added two new ldns functions ldns_key_assign_dsa_key and
ldns_key_assign_rsa_key and use those from ldns_key_new_frm_algorithm,
to maintain the API but prevent unnecessary allocations/frees.

Thanks for spotting this!

-- Willem

Op 11-07-12 19:12, Michael Sheldon schreef:
> There are two memory leaks in keys.c, ldns_key_new_frm_algorithm
> 
> The issue in the following is that ldns_key_set_rsa_key uses
> EVP_PKEY_set1_RSA(), which *copies* the RSA data, thus the original data
> must be freed by the calling application.
> 
> 00837                 case LDNS_SIGN_RSAMD5:
> 00838                 case LDNS_SIGN_RSASHA1:
> 00839                 case LDNS_SIGN_RSASHA1_NSEC3:
> 00840                 case LDNS_SIGN_RSASHA256:
> 00841                 case LDNS_SIGN_RSASHA512:
> 00842 #ifdef HAVE_SSL
> 00843                         r = RSA_generate_key((int)size, RSA_F4,
> NULL, NULL);
> 00844                         if(!r) {
> 00845                                 ldns_key_free(k);
> 00846                                 return NULL;
> 00847                         }
> 00848                         if (RSA_check_key(r) != 1) {
> 00849                                 ldns_key_free(k);
> 00850                                 return NULL;
> 00851                         }
> 00852                         ldns_key_set_rsa_key(k, r);
> 00853 #endif /* HAVE_SSL */
> 00854                         break;
> 
> The solution is to use RSA_free(r) after line 852.
> 
> The same issue applies in the following code. ldns_key_set_dsa_key uses
> EVP_PKEY_set1_DSA(), which *copies* the DSA data, thus the original data
> must be freed by the calling application.
> 
> 00855                 case LDNS_SIGN_DSA:
> 00856                 case LDNS_SIGN_DSA_NSEC3:
> 00857 #ifdef HAVE_SSL
> 00858                         d = DSA_generate_parameters((int)size,
> NULL, 0, NULL, NULL, NULL, NULL);
> 00859                         if (!d) {
> 00860                                 ldns_key_free(k);
> 00861                                 return NULL;
> 00862                         }
> 00863                         if (DSA_generate_key(d) != 1) {
> 00864                                 ldns_key_free(k);
> 00865                                 return NULL;
> 00866                         }
> 00867                         ldns_key_set_dsa_key(k, d);
> 00868 #endif /* HAVE_SSL */
> 00869                         break;
> 
> The solution is to use DSA_free(d) after line 867.
> 
> An alternative solution is to use EVP_PKEY_assign_RSA and
> EVP_PKEY_assign_DSA in place of EVP_PKEY_set1_RSA and EVP_PKEY_set1_DSA.
> The problem here is that it changes the existing behaviour of
> ldns_key_set_rsa_key and ldns_key_set_dsa_key, which developers may be
> calling.
> 
> Michael Sheldon
> Dev-DNS Services
> GoDaddy.com
> 
> 
> 
> _______________________________________________
> ldns-users mailing list
> ldns-users at open.nlnetlabs.nl
> http://open.nlnetlabs.nl/mailman/listinfo/ldns-users
>