Fix SRP memory leaks
authorMatt Caswell <matt@openssl.org>
Wed, 16 Sep 2015 09:24:37 +0000 (10:24 +0100)
committerMatt Caswell <matt@openssl.org>
Mon, 21 Sep 2015 09:23:25 +0000 (10:23 +0100)
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 <emilia@openssl.org>
(cherry picked from commit bf95cde28712cfcad90cb3975cdcb8e5c0f20fde)

crypto/srp/srp_vfy.c

index 50f75d7e4c9f16ba30385684114dc04859ebb867..a3f1a8a0a4d5bd00c4f665a318ce0742decd4e4c 100644 (file)
@@ -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;