Fix SRP memory leaks

[openssl.git] / crypto / srp / srp_vfy.c

diff --git a/crypto/srp/srp_vfy.c b/crypto/srp/srp_vfy.c
index 82b9a77b14f167634910b997060fb09c4b3b07d6..e81ae01779673a7061188d4b76da25ef1988e9da 100644 (file)
--- a/crypto/srp/srp_vfy.c
+++ b/crypto/srp/srp_vfy.c
@@ -58,7 +58,7 @@
  *
  */
 #ifndef OPENSSL_NO_SRP
-# include "cryptlib.h"
+# include "internal/cryptlib.h"
 # include <openssl/sha.h>
 # include <openssl/srp.h>
 # include <openssl/evp.h>
@@ -198,7 +198,7 @@ static void SRP_user_pwd_free(SRP_user_pwd *user_pwd)
 
 static SRP_user_pwd *SRP_user_pwd_new(void)
 {
-    SRP_user_pwd *ret = OPENSSL_malloc(sizeof(SRP_user_pwd));
+    SRP_user_pwd *ret = OPENSSL_malloc(sizeof(*ret));
     if (ret == NULL)
         return NULL;
     ret->N = NULL;
@@ -249,12 +249,12 @@ static int SRP_user_pwd_set_sv_BN(SRP_user_pwd *vinfo, BIGNUM *s, BIGNUM *v)
 
 SRP_VBASE *SRP_VBASE_new(char *seed_key)
 {
-    SRP_VBASE *vb = (SRP_VBASE *)OPENSSL_malloc(sizeof(SRP_VBASE));
+    SRP_VBASE *vb = OPENSSL_malloc(sizeof(*vb));
 
     if (vb == NULL)
         return NULL;
-    if (!(vb->users_pwd = sk_SRP_user_pwd_new_null()) ||
-        !(vb->gN_cache = sk_SRP_gN_cache_new_null())) {
+    if ((vb->users_pwd = sk_SRP_user_pwd_new_null()) == NULL
+        || (vb->gN_cache = sk_SRP_gN_cache_new_null()) == NULL) {
         OPENSSL_free(vb);
         return NULL;
     }
@@ -270,22 +270,22 @@ SRP_VBASE *SRP_VBASE_new(char *seed_key)
     return vb;
 }
 
-int SRP_VBASE_free(SRP_VBASE *vb)
+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);
     OPENSSL_free(vb->seed_key);
     OPENSSL_free(vb);
-    return 0;
 }
 
 static SRP_gN_cache *SRP_gN_new_init(const char *ch)
 {
     unsigned char tmp[MAX_LEN];
     int len;
+    SRP_gN_cache *newgN = OPENSSL_malloc(sizeof(*newgN));
 
-    SRP_gN_cache *newgN =
-        (SRP_gN_cache *)OPENSSL_malloc(sizeof(SRP_gN_cache));
     if (newgN == NULL)
         return NULL;
 
@@ -391,13 +391,14 @@ int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file)
              * we add this couple in the internal Stack
              */
 
-            if ((gN = (SRP_gN *) OPENSSL_malloc(sizeof(SRP_gN))) == NULL)
+            if ((gN = OPENSSL_malloc(sizeof(*gN))) == NULL)
                 goto err;
 
-            if (!(gN->id = BUF_strdup(pp[DB_srpid]))
-                || !(gN->N =
-                     SRP_gN_place_bn(vb->gN_cache, pp[DB_srpverifier]))
-                || !(gN->g = SRP_gN_place_bn(vb->gN_cache, pp[DB_srpsalt]))
+            if ((gN->id = BUF_strdup(pp[DB_srpid])) == NULL
+                || (gN->N = SRP_gN_place_bn(vb->gN_cache, pp[DB_srpverifier]))
+                        == NULL
+                || (gN->g = SRP_gN_place_bn(vb->gN_cache, pp[DB_srpsalt]))
+                        == NULL
                 || sk_SRP_gN_insert(SRP_gN_tab, gN, 0) == 0)
                 goto err;
 
@@ -458,8 +459,7 @@ int SRP_VBASE_init(SRP_VBASE *vb, char *verifier_file)
 
     SRP_user_pwd_free(user_pwd);
 
-    if (tmpdb)
-        TXT_DB_free(tmpdb);
+    TXT_DB_free(tmpdb);
     BIO_free_all(in);
 
     sk_SRP_gN_free(SRP_gN_tab);
@@ -510,7 +510,8 @@ SRP_user_pwd *SRP_VBASE_get_by_user(SRP_VBASE *vb, char *username)
          BN_bin2bn(digv, SHA_DIGEST_LENGTH, NULL)))
         return user;
 
- err:SRP_user_pwd_free(user);
+ err:
+    SRP_user_pwd_free(user);
     return NULL;
 }
 
@@ -521,22 +522,22 @@ 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))
         goto err;
 
     if (N) {
-        if (!(len = t_fromb64(tmp, N)))
+        if ((len = t_fromb64(tmp, N)) == 0)
             goto err;
         N_bn = BN_bin2bn(tmp, len, NULL);
-        if (!(len = t_fromb64(tmp, g)))
+        if ((len = t_fromb64(tmp, g)) == 0)
             goto err;
         g_bn = BN_bin2bn(tmp, len, NULL);
         defgNid = "*";
@@ -555,7 +556,7 @@ char *SRP_create_verifier(const char *user, const char *pass, char **salt,
 
         s = BN_bin2bn(tmp2, SRP_RANDOM_SALT_LEN, NULL);
     } else {
-        if (!(len = t_fromb64(tmp2, *salt)))
+        if ((len = t_fromb64(tmp2, *salt)) == 0)
             goto err;
         s = BN_bin2bn(tmp2, len, NULL);
     }
@@ -564,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:
@@ -587,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,
@@ -601,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) ||
@@ -612,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)
@@ -627,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;