BIO_new_from_core_bio: Fix heap-use-after-free after attach

The providers have to call up_ref to keep the cbio pointer, just like
the internal bio_prov.c does.

OSSL_STORE_attach passes a cbio pointer to the provider and then calls
ossl_core_bio_free(cbio). If up_ref is not called, the cbio gets
freed way too early.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15163)

diff --git a/crypto/bio/bss_core.c b/crypto/bio/bss_core.c
index 89b1ef73958995726c04c15f1467ab7434983585..de774e2b005f463f5409d3107cf72b1129e1c570 100644 (file)
--- a/crypto/bio/bss_core.c
+++ b/crypto/bio/bss_core.c
@@ -17,6 +17,8 @@ typedef struct {
     OSSL_FUNC_BIO_gets_fn *c_bio_gets;
     OSSL_FUNC_BIO_puts_fn *c_bio_puts;
     OSSL_FUNC_BIO_ctrl_fn *c_bio_ctrl;
+    OSSL_FUNC_BIO_up_ref_fn *c_bio_up_ref;
+    OSSL_FUNC_BIO_free_fn *c_bio_free;
 } BIO_CORE_GLOBALS;
 
 static void bio_core_globals_free(void *vbcg)
@@ -97,7 +99,10 @@ static int bio_core_new(BIO *bio)
 
 static int bio_core_free(BIO *bio)
 {
+    BIO_CORE_GLOBALS *bcgbl = get_globals(bio->libctx);
+
     BIO_set_init(bio, 0);
+    bcgbl->c_bio_free(BIO_get_data(bio));
 
     return 1;
 }
@@ -134,6 +139,10 @@ BIO *BIO_new_from_core_bio(OSSL_LIB_CTX *libctx, OSSL_CORE_BIO *corebio)
     if ((outbio = BIO_new_ex(libctx, BIO_s_core())) == NULL)
         return NULL;
 
+    if (!bcgbl->c_bio_up_ref(corebio)) {
+        BIO_free(outbio);
+        return NULL;
+    }
     BIO_set_data(outbio, corebio);
     return outbio;
 }
@@ -164,6 +173,14 @@ int ossl_bio_init_core(OSSL_LIB_CTX *libctx, const OSSL_DISPATCH *fns)
             if (bcgbl->c_bio_ctrl == NULL)
                 bcgbl->c_bio_ctrl = OSSL_FUNC_BIO_ctrl(fns);
             break;
+        case OSSL_FUNC_BIO_UP_REF:
+            if (bcgbl->c_bio_up_ref == NULL)
+                bcgbl->c_bio_up_ref = OSSL_FUNC_BIO_up_ref(fns);
+            break;
+        case OSSL_FUNC_BIO_FREE:
+            if (bcgbl->c_bio_free == NULL)
+                bcgbl->c_bio_free = OSSL_FUNC_BIO_free(fns);
+            break;
         }
     }
 

diff --git a/test/bio_core_test.c b/test/bio_core_test.c
index ae326cef9290a4c1ab5c414b6294d06cb78f0459..77e846735f76dbb8141df68faf76d2e6ae040935 100644 (file)
--- a/test/bio_core_test.c
+++ b/test/bio_core_test.c
@@ -43,12 +43,24 @@ static long tst_bio_core_ctrl(OSSL_CORE_BIO *bio, int cmd, long num, void *ptr)
     return BIO_ctrl(bio->bio, cmd, num, ptr);
 }
 
+static int tst_bio_core_up_ref(OSSL_CORE_BIO *bio)
+{
+    return BIO_up_ref(bio->bio);
+}
+
+static int tst_bio_core_free(OSSL_CORE_BIO *bio)
+{
+    return BIO_free(bio->bio);
+}
+
 static const OSSL_DISPATCH biocbs[] = {
     { OSSL_FUNC_BIO_READ_EX, (void (*)(void))tst_bio_core_read_ex },
     { OSSL_FUNC_BIO_WRITE_EX, (void (*)(void))tst_bio_core_write_ex },
     { OSSL_FUNC_BIO_GETS, (void (*)(void))tst_bio_core_gets },
     { OSSL_FUNC_BIO_PUTS, (void (*)(void))tst_bio_core_puts },
     { OSSL_FUNC_BIO_CTRL, (void (*)(void))tst_bio_core_ctrl },
+    { OSSL_FUNC_BIO_UP_REF, (void (*)(void))tst_bio_core_up_ref },
+    { OSSL_FUNC_BIO_FREE, (void (*)(void))tst_bio_core_free },
     { 0, NULL }
 };