Fix a use-after-free in quic_tls.c

The comments in quic_tls.c claimed that the dummybio was never used by
us. In fact that is not entirely correct since we set and cleared the
retry flags on it. This means that we have to manage it properly, and update
it in the event of set1_bio() call on the record layer method.

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

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index 8fd02fba75409d6f0064c0d7999d316566941c44..70c67be8aa3acce2048de6e9344897260ec10f55 100644 (file)
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -84,6 +84,8 @@ struct ossl_record_layer_st {
     void *cbarg;
 };
 
+static int quic_set1_bio(OSSL_RECORD_LAYER *rl, BIO *bio);
+
 static int
 quic_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
                       int role, int direction, int level, uint16_t epoch,
@@ -111,7 +113,10 @@ quic_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
 
     rl->qtls = (QUIC_TLS *)rlarg;
     rl->level = level;
-    rl->dummybio = transport;
+    if (!quic_set1_bio(rl, transport)) {
+        QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
     rl->cbarg = cbarg;
     *retrl = rl;
 
@@ -193,6 +198,7 @@ static int quic_free(OSSL_RECORD_LAYER *rl)
     if (rl == NULL)
         return 1;
 
+    BIO_free(rl->dummybio);
     OPENSSL_free(rl);
     return 1;
 }
@@ -524,10 +530,11 @@ static int quic_free_buffers(OSSL_RECORD_LAYER *rl)
 
 static int quic_set1_bio(OSSL_RECORD_LAYER *rl, BIO *bio)
 {
-    /*
-     * Can be called to set the buffering BIO - which is then never used by us.
-     * We ignore it
-     */
+    if (bio != NULL && !BIO_up_ref(bio))
+        return 0;
+    BIO_free(rl->dummybio);
+    rl->dummybio = bio;
+
     return 1;
 }