Freeing of the values by the caller is not the issue. The issue is RSA_set0_key requires n and e to be none NULL. It the caller use RSA_get0_key to find the n and e then calculates a new d, than calls RSA_set0_key with the the same n and e pointers and the new d. RSA_set0_key will free n and e, and replace the pointer with the same pointer which just got freed.
An untested patch for rsa_lib.c is attached DSA has the same problems. Are there other new modules that may have the same issue? On 4/25/2016 8:08 AM, Richard Levitte via RT wrote:
In message <6b097acbe9d94724ac545f2529e45...@usma1ex-dag1mb1.msg.corp.akamai.com> on Mon, 25 Apr 2016 11:38:47 +0000, "Salz, Rich" <rs...@akamai.com> said: rsalz> > If nothing else, all the RSA_set0 routines should test if the same pointer rsalz> > value is being replaced if so do not free it. rsalz> > rsalz> > The same logic need to be done for all the RSA_set0_* functions as well as rsalz> > the DSA_set0_* functions. rsalz> rsalz> That seems like a bug we should fix. No, it's by design: : ; perldoc doc/crypto/RSA_get0_key.pod ... The n, e and d parameter values can be set by calling RSA_set0_key() and passing the new values for n, e and d as parameters to the function. Calling this function transfers the memory management of the values to the RSA object, and therefore the values that have been passed in should not be freed by the caller after this function has been called. ... : ; perldoc doc/crypto/DSA_get0_pqg.pod ... The p, q and g values can be set by calling DSA_set0_pqg() and passing the new values for p, q and g as parameters to the function. Calling this function transfers the memory management of the values to the DSA object, and therefore the values that have been passed in should not be freed directly after this function has been called. ... Cheers, Richard
-- Douglas E. Engert <deeng...@gmail.com>
diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c index 7ee575d..f106cf4 100644 --- a/crypto/rsa/rsa_lib.c +++ b/crypto/rsa/rsa_lib.c @@ -290,9 +290,12 @@ int RSA_set0_key(RSA *r, BIGNUM *n, BIGNUM *e, BIGNUM *d) if (n == NULL || e == NULL) return 0; - BN_free(r->n); - BN_free(r->e); - BN_free(r->d); + if (r->n != n) + BN_free(r->n); + if (r->e != e) + BN_free(r->e); + if (r->d != d) + BN_free(r->d); r->n = n; r->e = e; r->d = d; @@ -305,8 +308,10 @@ int RSA_set0_factors(RSA *r, BIGNUM *p, BIGNUM *q) if (p == NULL || q == NULL) return 0; - BN_free(r->p); - BN_free(r->q); + if (r->p != p) + BN_free(r->p); + if (r->q != q) + BN_free(r->q); r->p = p; r->q = q; @@ -318,9 +323,12 @@ int RSA_set0_crt_params(RSA *r, BIGNUM *dmp1, BIGNUM *dmq1, BIGNUM *iqmp) if (dmp1 == NULL || dmq1 == NULL || iqmp == NULL) return 0; - BN_free(r->dmp1); - BN_free(r->dmq1); - BN_free(r->iqmp); + if (r->dmp1 != dmp1) + BN_free(r->dmp1); + if (r->dmq1 != dmq1) + BN_free(r->dmq1); + if (r->iqmp != iqmp) + BN_free(r->iqmp); r->dmp1 = dmp1; r->dmq1 = dmq1; r->iqmp = iqmp;
-- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev