Update provider_util.c to correctly handle ENGINE references
authorMatt Caswell <matt@openssl.org>
Fri, 15 Oct 2021 15:28:53 +0000 (16:28 +0100)
committerMatt Caswell <matt@openssl.org>
Tue, 19 Oct 2021 15:27:14 +0000 (16:27 +0100)
provider_util.c failed to free ENGINE references when clearing a cipher
or a digest. Additionally ciphers and digests were not copied correctly,
which would lead to double-frees if it were not for the previously
mentioned leaks.

Fixes #16845

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16846)

(cherry picked from commit 86c15ba87488f88e6191f098ff154f79ce91847b)

providers/common/provider_util.c

index fcfbab632d5bcef0d3df1f136744f0141302277d..58d4db33793f5ca8ae9904abe781ba4f4309fef5 100644 (file)
@@ -26,6 +26,9 @@ void ossl_prov_cipher_reset(PROV_CIPHER *pc)
     EVP_CIPHER_free(pc->alloc_cipher);
     pc->alloc_cipher = NULL;
     pc->cipher = NULL;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(pc->engine);
+#endif
     pc->engine = NULL;
 }
 
@@ -33,6 +36,12 @@ int ossl_prov_cipher_copy(PROV_CIPHER *dst, const PROV_CIPHER *src)
 {
     if (src->alloc_cipher != NULL && !EVP_CIPHER_up_ref(src->alloc_cipher))
         return 0;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    if (src->engine != NULL && !ENGINE_init(src->engine)) {
+        EVP_CIPHER_free(src->alloc_cipher);
+        return 0;
+    }
+#endif
     dst->engine = src->engine;
     dst->cipher = src->cipher;
     dst->alloc_cipher = src->alloc_cipher;
@@ -52,6 +61,9 @@ static int load_common(const OSSL_PARAM params[], const char **propquery,
         *propquery = p->data;
     }
 
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(*engine);
+#endif
     *engine = NULL;
     /* Inside the FIPS module, we don't support legacy ciphers */
 #if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
@@ -59,10 +71,18 @@ static int load_common(const OSSL_PARAM params[], const char **propquery,
     if (p != NULL) {
         if (p->data_type != OSSL_PARAM_UTF8_STRING)
             return 0;
-        ENGINE_finish(*engine);
+        /* Get a structural reference */
         *engine = ENGINE_by_id(p->data);
         if (*engine == NULL)
             return 0;
+        /* Get a functional reference */
+        if (!ENGINE_init(*engine)) {
+            ENGINE_free(*engine);
+            *engine = NULL;
+            return 0;
+        }
+        /* Free the structural reference */
+        ENGINE_free(*engine);
     }
 #endif
     return 1;
@@ -122,6 +142,9 @@ void ossl_prov_digest_reset(PROV_DIGEST *pd)
     EVP_MD_free(pd->alloc_md);
     pd->alloc_md = NULL;
     pd->md = NULL;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(pd->engine);
+#endif
     pd->engine = NULL;
 }
 
@@ -129,6 +152,12 @@ int ossl_prov_digest_copy(PROV_DIGEST *dst, const PROV_DIGEST *src)
 {
     if (src->alloc_md != NULL && !EVP_MD_up_ref(src->alloc_md))
         return 0;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    if (src->engine != NULL && !ENGINE_init(src->engine)) {
+        EVP_MD_free(src->alloc_md);
+        return 0;
+    }
+#endif
     dst->engine = src->engine;
     dst->md = src->md;
     dst->alloc_md = src->alloc_md;