Skip to content

Commit

Permalink
Fix a use-after-free in quic_tls.c
Browse files Browse the repository at this point in the history
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 #21686)
  • Loading branch information
mattcaswell committed Aug 15, 2023
1 parent 644ef0b commit 643f542
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions ssl/quic/quic_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 643f542

Please sign in to comment.