Avoid double-free errors in RSA key generation (#525)

* Avoid double-free of pub_exp (OpenSSL) * Avoid double-free of pub_exp (BoringSSL) We should not free pub_exp after we have successfully passed it into EVP_PKEY_CTX with EVP_PKEY_CTX_ctrl(EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP) call. Otherwise we'll free the same object twice when calling EVP_PKEY_CTX_free(). * Avoid double-free of EVP_PKEY_CTX (BoringSSL) We do not own pkey_ctx in soter_rsa_gen_key(). It is owned by the caller: soter_rsa_key_pair_gen_init(). We should not free it ourselves in case of errors. (And we should not use magic macros unless really necessary.)
cossacklabs · Sep 5, 2019 · ffb17b9 · ffb17b9
1 parent 4d275a2
commit ffb17b9
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 7 deletions.
diff --git a/src/soter/boringssl/soter_rsa_common.c b/src/soter/boringssl/soter_rsa_common.c
@@ -69,12 +69,15 @@ soter_status_t soter_rsa_gen_key(EVP_PKEY_CTX* pkey_ctx, const unsigned key_leng
         return SOTER_FAIL;
     }
 
-    SOTER_IF_FAIL(1 <= EVP_PKEY_CTX_set_rsa_keygen_pubexp(pkey_ctx, pub_exp),
-                  (BN_free(pub_exp), EVP_PKEY_CTX_free(pkey_ctx)));
+    if (1 > EVP_PKEY_CTX_set_rsa_keygen_pubexp(pkey_ctx, pub_exp)) {
+        BN_free(pub_exp);
+        return SOTER_FAIL;
+    }
     /* Override default key size for RSA key. Currently OpenSSL has default key size of 1024.
      * LibreSSL has 2048. We will put 2048 explicitly */
-    SOTER_IF_FAIL((1 <= EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, rsa_key_length(key_length))),
-                  (BN_free(pub_exp), EVP_PKEY_CTX_free(pkey_ctx)));
+    if (1 > EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, rsa_key_length(key_length))) {
+        return SOTER_FAIL;
+    }
 
     if (!EVP_PKEY_keygen(pkey_ctx, &pkey)) {
         return SOTER_FAIL;

diff --git a/src/soter/openssl/soter_rsa_key_pair_gen.c b/src/soter/openssl/soter_rsa_key_pair_gen.c
@@ -76,9 +76,8 @@ soter_status_t soter_rsa_key_pair_gen_init(soter_rsa_key_pair_gen_t* ctx, const
                                           EVP_PKEY_CTRL_RSA_KEYGEN_BITS,
                                           rsa_key_length(key_length),
                                           NULL)),
-                  (BN_free(pub_exp), EVP_PKEY_CTX_free(ctx->pkey_ctx)));
-    SOTER_IF_FAIL(EVP_PKEY_keygen(ctx->pkey_ctx, &pkey),
-                  (BN_free(pub_exp), EVP_PKEY_CTX_free(ctx->pkey_ctx)));
+                  (EVP_PKEY_CTX_free(ctx->pkey_ctx)));
+    SOTER_IF_FAIL(EVP_PKEY_keygen(ctx->pkey_ctx, &pkey), (EVP_PKEY_CTX_free(ctx->pkey_ctx)));
     return SOTER_SUCCESS;
 }