From 0110259f05f58e9444a3744c8bf4d92c71b4783f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 16 Sep 2015 10:24:37 +0100 Subject: Fix SRP memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit There were some memory leaks in the creation of an SRP verifier (both on successful completion and also on some error paths). Reviewed-by: Emilia Käsper (cherry picked from commit bf95cde28712cfcad90cb3975cdcb8e5c0f20fde) --- crypto/srp/srp_vfy.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/crypto/srp/srp_vfy.c b/crypto/srp/srp_vfy.c index 50f75d7..a3f1a8a 100644 --- a/crypto/srp/srp_vfy.c +++ b/crypto/srp/srp_vfy.c @@ -521,12 +521,12 @@ char *SRP_create_verifier(const char *user, const char *pass, char **salt, char **verifier, const char *N, const char *g) { int len; - char *result = NULL; - char *vf; + char *result = NULL, *vf = NULL; BIGNUM *N_bn = NULL, *g_bn = NULL, *s = NULL, *v = NULL; unsigned char tmp[MAX_LEN]; unsigned char tmp2[MAX_LEN]; char *defgNid = NULL; + int vfsize = 0; if ((user == NULL) || (pass == NULL) || (salt == NULL) || (verifier == NULL)) @@ -564,22 +564,23 @@ char *SRP_create_verifier(const char *user, const char *pass, char **salt, goto err; BN_bn2bin(v, tmp); - if (((vf = OPENSSL_malloc(BN_num_bytes(v) * 2)) == NULL)) + vfsize = BN_num_bytes(v) * 2; + if (((vf = OPENSSL_malloc(vfsize)) == NULL)) goto err; t_tob64(vf, tmp, BN_num_bytes(v)); - *verifier = vf; if (*salt == NULL) { char *tmp_salt; if ((tmp_salt = OPENSSL_malloc(SRP_RANDOM_SALT_LEN * 2)) == NULL) { - OPENSSL_free(vf); goto err; } t_tob64(tmp_salt, tmp2, SRP_RANDOM_SALT_LEN); *salt = tmp_salt; } + *verifier = vf; + vf = NULL; result = defgNid; err: @@ -587,11 +588,21 @@ char *SRP_create_verifier(const char *user, const char *pass, char **salt, BN_free(N_bn); BN_free(g_bn); } + OPENSSL_cleanse(vf, vfsize); + OPENSSL_free(vf); + BN_clear_free(s); + BN_clear_free(v); return result; } /* - * create a verifier (*salt,*verifier,g and N are BIGNUMs) + * create a verifier (*salt,*verifier,g and N are BIGNUMs). If *salt != NULL + * then the provided salt will be used. On successful exit *verifier will point + * to a newly allocated BIGNUM containing the verifier and (if a salt was not + * provided) *salt will be populated with a newly allocated BIGNUM containing a + * random salt. + * The caller is responsible for freeing the allocated *salt and *verifier + * BIGNUMS. */ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, BIGNUM **verifier, BIGNUM *N, BIGNUM *g) @@ -600,6 +611,7 @@ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, BIGNUM *x = NULL; BN_CTX *bn_ctx = BN_CTX_new(); unsigned char tmp2[MAX_LEN]; + BIGNUM *salttmp = NULL; if ((user == NULL) || (pass == NULL) || @@ -614,10 +626,12 @@ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, if (RAND_pseudo_bytes(tmp2, SRP_RANDOM_SALT_LEN) < 0) goto err; - *salt = BN_bin2bn(tmp2, SRP_RANDOM_SALT_LEN, NULL); + salttmp = BN_bin2bn(tmp2, SRP_RANDOM_SALT_LEN, NULL); + } else { + salttmp = *salt; } - x = SRP_Calc_x(*salt, user, pass); + x = SRP_Calc_x(salttmp, user, pass); *verifier = BN_new(); if (*verifier == NULL) @@ -631,9 +645,11 @@ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, srp_bn_print(*verifier); result = 1; + *salt = salttmp; err: - + if (*salt != salttmp) + BN_clear_free(salttmp); BN_clear_free(x); BN_CTX_free(bn_ctx); return result; -- 1.7.12.1