From bf95cde28712cfcad90cb3975cdcb8e5c0f20fde Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 16 Sep 2015 10:24:37 +0100 Subject: [PATCH] Fix SRP memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 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 --- crypto/srp/srp_vfy.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/crypto/srp/srp_vfy.c b/crypto/srp/srp_vfy.c index 39c89e8d88..e81ae01779 100644 --- a/crypto/srp/srp_vfy.c +++ b/crypto/srp/srp_vfy.c @@ -522,12 +522,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)) @@ -565,22 +565,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: @@ -588,11 +589,20 @@ char *SRP_create_verifier(const char *user, const char *pass, char **salt, BN_free(N_bn); BN_free(g_bn); } + OPENSSL_clear_free(vf, vfsize); + 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, const BIGNUM *N, @@ -602,6 +612,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) || @@ -613,10 +624,12 @@ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, if (RAND_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) @@ -628,9 +641,11 @@ int SRP_create_verifier_BN(const char *user, const char *pass, BIGNUM **salt, } result = 1; + *salt = salttmp; err: - + if (*salt != salttmp) + BN_clear_free(salttmp); BN_clear_free(x); BN_CTX_free(bn_ctx); return result; -- 2.25.1