EVP: fix memleak in evp_pkey_downgrade()
authorRichard Levitte <levitte@openssl.org>
Sat, 11 Apr 2020 11:16:22 +0000 (13:16 +0200)
committerRichard Levitte <levitte@openssl.org>
Wed, 15 Apr 2020 09:04:35 +0000 (11:04 +0200)
The EVP_KEYMGMT pointer in the pkey is removed when downgrading, but
wasn't necessarily freed when need, thus leaving an incorrect
reference count.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11328)

crypto/evp/p_lib.c

index c1a8a88..9f04c72 100644 (file)
@@ -1559,8 +1559,11 @@ int evp_pkey_downgrade(EVP_PKEY *pk)
     evp_pkey_free_it(pk);
     if (EVP_PKEY_set_type(pk, type)) {
         /* If the key is typed but empty, we're done */
     evp_pkey_free_it(pk);
     if (EVP_PKEY_set_type(pk, type)) {
         /* If the key is typed but empty, we're done */
-        if (keydata == NULL)
+        if (keydata == NULL) {
+            /* We're dropping the EVP_KEYMGMT */
+            EVP_KEYMGMT_free(keymgmt);
             return 1;
             return 1;
+        }
 
         if (pk->ameth->import_from == NULL) {
             ERR_raise_data(ERR_LIB_EVP, EVP_R_NO_IMPORT_FUNCTION,
 
         if (pk->ameth->import_from == NULL) {
             ERR_raise_data(ERR_LIB_EVP, EVP_R_NO_IMPORT_FUNCTION,
@@ -1579,6 +1582,9 @@ int evp_pkey_downgrade(EVP_PKEY *pk)
 
             /* Synchronize the dirty count */
             pk->dirty_cnt_copy = pk->ameth->dirty_cnt(pk);
 
             /* Synchronize the dirty count */
             pk->dirty_cnt_copy = pk->ameth->dirty_cnt(pk);
+
+            /* evp_keymgmt_export() increased the refcount... */
+            EVP_KEYMGMT_free(keymgmt);
             return 1;
         }
 
             return 1;
         }
 
@@ -1597,6 +1603,8 @@ int evp_pkey_downgrade(EVP_PKEY *pk)
         ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
         return 0;
     }
         ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
         return 0;
     }
+    /* EVP_PKEY_set_type_by_keymgmt() increased the refcount... */
+    EVP_KEYMGMT_free(keymgmt);
     pk->keydata = keydata;
     evp_keymgmt_util_cache_keyinfo(pk);
     return 0;     /* No downgrade, but at least the key is restored */
     pk->keydata = keydata;
     evp_keymgmt_util_cache_keyinfo(pk);
     return 0;     /* No downgrade, but at least the key is restored */