bignum: make bignum_free safer

8 files changed, 34 insertions, 30 deletions

diff --git a/include/libssh/libcrypto.h b/include/libssh/libcrypto.h
index 2f8099ee..428f0495 100644
--- a/include/libssh/libcrypto.h
+++ b/include/libssh/libcrypto.h
@@ -67,7 +67,12 @@ typedef BIGNUM*  bignum;
 typedef BN_CTX* bignum_CTX;
 
 #define bignum_new() BN_new()
-#define bignum_free(num) BN_clear_free(num)
+#define bignum_safe_free(num) do { \
+    if ((num) != NULL) { \
+        BN_clear_free((num)); \
+        (num)=NULL; \
+    } \
+    } while(0)
 #define bignum_set_word(bn,n) BN_set_word(bn,n)
 #define bignum_bin2bn(data,datalen,dest) do { \
     (*dest)=BN_new(); \
diff --git a/include/libssh/libgcrypt.h b/include/libssh/libgcrypt.h
index b950b3bd..d2bb7130 100644
--- a/include/libssh/libgcrypt.h
+++ b/include/libssh/libgcrypt.h
@@ -57,7 +57,12 @@ int ssh_gcry_dec2bn(bignum *bn, const char *data);
 char *ssh_gcry_bn2dec(bignum bn);
 
 #define bignum_new() gcry_mpi_new(0)
-#define bignum_free(num) gcry_mpi_release(num)
+#define bignum_safe_free(num) do { \
+    if ((num) != NULL) { \
+        gcry_mpi_release((num)); \
+        (num)=NULL; \
+    } \
+    } while (0)
 #define bignum_ctx_new() NULL
 #define bignum_ctx_free(num) do {(num) = NULL;} while(0)
 #define bignum_ctx_invalid(ctx) 0
diff --git a/src/dh.c b/src/dh.c
index 19031da4..42b7d137 100644
--- a/src/dh.c
+++ b/src/dh.c
@@ -136,16 +136,13 @@ int ssh_dh_init(void) {
 
     bignum_bin2bn(p_group1_value, P_GROUP1_LEN, &p_group1);
     if (p_group1 == NULL) {
-      bignum_free(g);
-      g = NULL;
+      bignum_safe_free(g);
       return SSH_ERROR;
     }
     bignum_bin2bn(p_group14_value, P_GROUP14_LEN, &p_group14);
     if (p_group14 == NULL) {
-      bignum_free(g);
-      bignum_free(p_group1);
-      g = NULL;
-      p_group1 = NULL;
+      bignum_safe_free(g);
+      bignum_safe_free(p_group1);
       return SSH_ERROR;
     }
 
@@ -161,12 +158,9 @@ int ssh_dh_init(void) {
  */
 void ssh_dh_finalize(void) {
   if (dh_crypto_initialized) {
-    bignum_free(g);
-    g = NULL;
-    bignum_free(p_group1);
-    p_group1 = NULL;
-    bignum_free(p_group14);
-    p_group14 = NULL;
+    bignum_safe_free(g);
+    bignum_safe_free(p_group1);
+    bignum_safe_free(p_group14);
     dh_crypto_initialized=0;
   }
 }
diff --git a/src/gcrypt_missing.c b/src/gcrypt_missing.c
index e07843b8..fa095442 100644
--- a/src/gcrypt_missing.c
+++ b/src/gcrypt_missing.c
@@ -70,7 +70,7 @@ char *ssh_gcry_bn2dec(bignum bn) {
     num = bignum_new();
     if (num == NULL) {
       SAFE_FREE(ret);
-      bignum_free(ten);
+      bignum_safe_free(ten);
       return NULL;
     }
 
@@ -89,9 +89,9 @@ char *ssh_gcry_bn2dec(bignum bn) {
       ret[count2] = ret[count2 + count];
     }
     ret[count2] = 0;
-    bignum_free(num);
-    bignum_free(bndup);
-    bignum_free(ten);
+    bignum_safe_free(num);
+    bignum_safe_free(bndup);
+    bignum_safe_free(ten);
   }
 
   return ret;
diff --git a/src/kex1.c b/src/kex1.c
index 16919847..96147515 100644
--- a/src/kex1.c
+++ b/src/kex1.c
@@ -127,8 +127,8 @@ static int modulus_smaller(ssh_public_key k1, ssh_public_key k2){
     else
         res=0;
 #ifdef HAVE_LIBGCRYPT
-    bignum_free(n1);
-    bignum_free(n2);
+    bignum_safe_free(n1);
+    bignum_safe_free(n2);
 #endif
     return res;
     
diff --git a/src/known_hosts.c b/src/known_hosts.c
index a9788213..44d9f42b 100644
--- a/src/known_hosts.c
+++ b/src/known_hosts.c
@@ -224,17 +224,17 @@ static int check_public_key(ssh_session session, char **tokens) {
       tmpstring = malloc(4 + len);
       if (tmpstring == NULL) {
         ssh_buffer_free(pubkey_buffer);
-        bignum_free(tmpbn);
+        bignum_safe_free(tmpbn);
         return -1;
       }
       /* TODO: fix the hardcoding */
       tmpstring->size = htonl(len);
       bignum_bn2bin(tmpbn, len, ssh_string_data(tmpstring));
-      bignum_free(tmpbn);
+      bignum_safe_free(tmpbn);
       if (ssh_buffer_add_ssh_string(pubkey_buffer, tmpstring) < 0) {
         ssh_buffer_free(pubkey_buffer);
         ssh_string_free(tmpstring);
-        bignum_free(tmpbn);
+        bignum_safe_free(tmpbn);
         return -1;
       }
       ssh_string_free(tmpstring);
diff --git a/src/pki_crypto.c b/src/pki_crypto.c
index 71eacc8b..eeaffd94 100644
--- a/src/pki_crypto.c
+++ b/src/pki_crypto.c
@@ -1448,7 +1448,7 @@ ssh_signature pki_signature_from_blob(const ssh_key pubkey,
 #ifdef DEBUG_CRYPTO
                 ssh_print_hexa("r", ssh_string_data(r), ssh_string_len(r));
 #endif
-                bignum_free(sig->ecdsa_sig->r);
+                bignum_safe_free(sig->ecdsa_sig->r);
                 bignum_bin2bn(ssh_string_data(r), ssh_string_len(r), &sig->ecdsa_sig->r);
                 ssh_string_burn(r);
                 ssh_string_free(r);
@@ -1470,7 +1470,7 @@ ssh_signature pki_signature_from_blob(const ssh_key pubkey,
                 ssh_print_hexa("s", ssh_string_data(s), ssh_string_len(s));
 #endif
 
-                bignum_free(sig->ecdsa_sig->s);
+                bignum_safe_free(sig->ecdsa_sig->s);
                 bignum_bin2bn(ssh_string_data(s), ssh_string_len(s), &sig->ecdsa_sig->s);
                 ssh_string_burn(s);
                 ssh_string_free(s);
diff --git a/src/wrapper.c b/src/wrapper.c
index 075b4fd0..91ed415b 100644
--- a/src/wrapper.c
+++ b/src/wrapper.c
@@ -155,11 +155,11 @@ void crypto_free(struct ssh_crypto_struct *crypto){
   cipher_free(crypto->in_cipher);
   cipher_free(crypto->out_cipher);
 
-  bignum_free(crypto->e);
-  bignum_free(crypto->f);
-  bignum_free(crypto->x);
-  bignum_free(crypto->y);
-  bignum_free(crypto->k);
+  bignum_safe_free(crypto->e);
+  bignum_safe_free(crypto->f);
+  bignum_safe_free(crypto->x);
+  bignum_safe_free(crypto->y);
+  bignum_safe_free(crypto->k);
 #ifdef HAVE_ECDH
   SAFE_FREE(crypto->ecdh_client_pubkey);
   SAFE_FREE(crypto->ecdh_server_pubkey);