Re: [openssl-dev] [openssl.org #4172] SRP VBASE stuff still leaking memory

[Kurt Roeckx via RT]

On Mon, Dec 07, 2015 at 03:47:56PM +0000, Michel via RT wrote:
> Hi,
> 
> Following my previous mail, here attached is an updated patch against 1.02e
> to fix the SRP VBASE memory leaks.

Can you confirm that this would be the correct patch for master?

I still need to look at it.


Kurt



diff --git a/crypto/srp/srp_vfy.c b/crypto/srp/srp_vfy.c
index 1be68f2..dc649c8 100644
--- a/crypto/srp/srp_vfy.c
+++ b/crypto/srp/srp_vfy.c
@@ -275,7 +275,7 @@ void SRP_VBASE_free(SRP_VBASE *vb)
     if (!vb)
         return;
     sk_SRP_user_pwd_pop_free(vb->users_pwd, SRP_user_pwd_free);
-    sk_SRP_gN_cache_free(vb->gN_cache);
+    sk_SRP_gN_cache_pop_free(vb->gN_cache, SRP_gN_cache_free);
     OPENSSL_free(vb->seed_key);
     OPENSSL_free(vb);
 }
@@ -302,13 +302,14 @@ static SRP_gN_cache *SRP_gN_new_init(const char *ch)
     return NULL;
 }
 
-static void SRP_gN_free(SRP_gN_cache *gN_cache)
+static void SRP_gN_free(SRP_gN *gN)
 {
-    if (gN_cache == NULL)
+    if (gN == NULL)
         return;
-    OPENSSL_free(gN_cache->b64_bn);
-    BN_free(gN_cache->bn);
-    OPENSSL_free(gN_cache);
+    OPENSSL_free(gN->id);
+    BN_free(gN->g);
+    BN_free(gN->N);
+    OPENSSL_free(gN);
 }
 
 static SRP_gN *SRP_get_gN_by_id(const char *id, STACK_OF(SRP_gN) *gN_tab)
@@ -343,7 +344,7 @@ static BIGNUM *SRP_gN_place_bn(STACK_OF(SRP_gN_cache) *gN_cache, char *ch)
         if (newgN) {
             if (sk_SRP_gN_cache_insert(gN_cache, newgN, 0) > 0)
                 return newgN->bn;
-            SRP_gN_free(newgN);
+            SRP_gN_cache_free(newgN);
         }
     }
     return NULL;
@@ -391,7 +392,7 @@ int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file)
              * we add this couple in the internal Stack
              */
 
-            if ((gN = OPENSSL_malloc(sizeof(*gN))) == NULL)
+            if ((gN = OPENSSL_zalloc(sizeof(*gN))) == NULL)
                 goto err;
 
             if ((gN->id = BUF_strdup(pp[DB_srpid])) == NULL
@@ -447,21 +448,16 @@ int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file)
     error_code = SRP_NO_ERROR;
 
  err:
-    /*
-     * there may be still some leaks to fix, if this fails, the application
-     * terminates most likely
-     */
-
-    if (gN != NULL) {
-        OPENSSL_free(gN->id);
-        OPENSSL_free(gN);
-    }
-
+    SRP_gN_free(gN);
     SRP_user_pwd_free(user_pwd);
 
     TXT_DB_free(tmpdb);
     BIO_free_all(in);
 
+    for (i=0; i < sk_SRP_gN_num(SRP_gN_tab); i++) {
+        OPENSSL_free(sk_SRP_gN_value(SRP_gN_tab, i)->id);
+        OPENSSL_free(sk_SRP_gN_value(SRP_gN_tab, i));
+    }
     sk_SRP_gN_free(SRP_gN_tab);
 
     return error_code;

_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev