Re: [openssl.org #3226] [PATCH] crypto/srp/srp_lib.c: add/correct some error handling

[Florian Zumbiehl via RT]

Hi,

> With this patch, I am afraid in case of error, the context will not
> be cleaned up.
> Shouldn't the line :
> EVP_MD_CTX_cleanup(&ctxt);
> be moved inside the 'err:' block ?

Good catch!

New version below ...

Regards, Florian
---

diff --git a/crypto/srp/srp_lib.c b/crypto/srp/srp_lib.c
index 18d1bb9..782f23a 100644
--- a/crypto/srp/srp_lib.c
+++ b/crypto/srp/srp_lib.c
@@ -82,6 +82,7 @@
 static BIGNUM *srp_Calc_k(BIGNUM *N, BIGNUM *g)
        {
        /* k = SHA1(N | PAD(g)) -- tls-srp draft 8 */
+       BIGNUM *ret = NULL;
 
        unsigned char digest[SHA_DIGEST_LENGTH];
        unsigned char *tmp;
@@ -94,19 +95,26 @@ static BIGNUM *srp_Calc_k(BIGNUM *N, BIGNUM *g)
        BN_bn2bin(N,tmp) ;
 
        EVP_MD_CTX_init(&ctxt);
-       EVP_DigestInit_ex(&ctxt, EVP_sha1(), NULL);
-       EVP_DigestUpdate(&ctxt, tmp, longN);
+       if (!EVP_DigestInit_ex(&ctxt, EVP_sha1(), NULL))
+               goto err;
+       if (!EVP_DigestUpdate(&ctxt, tmp, longN))
+               goto err;
 
        memset(tmp, 0, longN);
        longg = BN_bn2bin(g,tmp) ;
         /* use the zeros behind to pad on left */
-       EVP_DigestUpdate(&ctxt, tmp + longg, longN-longg);
-       EVP_DigestUpdate(&ctxt, tmp, longg);
-       OPENSSL_free(tmp);
+       if (!EVP_DigestUpdate(&ctxt, tmp + longg, longN-longg))
+               goto err;
+       if (!EVP_DigestUpdate(&ctxt, tmp, longg))
+               goto err;
 
-       EVP_DigestFinal_ex(&ctxt, digest, NULL);
+       if (!EVP_DigestFinal_ex(&ctxt, digest, NULL))
+               goto err;
+       ret = BN_bin2bn(digest, sizeof(digest), NULL);  
+err:
        EVP_MD_CTX_cleanup(&ctxt);
-       return BN_bin2bn(digest, sizeof(digest), NULL); 
+       OPENSSL_free(tmp);
+       return ret;
        }
 
 BIGNUM *SRP_Calc_u(BIGNUM *A, BIGNUM *B, BIGNUM *N)
@@ -287,7 +295,12 @@ BIGNUM *SRP_Calc_client_key(BIGNUM *N, BIGNUM *B, BIGNUM 
*g, BIGNUM *x, BIGNUM *
        if (!BN_mod_exp(K,tmp,tmp2,N,bn_ctx))
                goto err;
 
+       if (0)
+               {
 err :
+               BN_clear_free(K);
+               K = NULL;
+               }
        BN_CTX_free(bn_ctx);
        BN_clear_free(tmp);
        BN_clear_free(tmp2);
-- 


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@openssl.org
Automated List Manager                           majord...@openssl.org