From 840af0928b15bd398c3e945cb7c44f50f7c38780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Mon, 13 Nov 2023 09:06:59 +0100 Subject: [PATCH] BUG/MEDIUM: quic: Non initialized CRYPTO data stream deferencing This bug arrived with this commit: BUG/MINOR: quic: Useless use of non-contiguous buffer for in order CRYPTO data Before this commit qc->cstream was tested before entering qc_treat_rx_crypto_frms(). This patch restablishes this behavior. Furthermore, it simplyfies qc_ssl_provide_all_quic_data() which is a little bit ugly: the CRYPTO data frame may be freed asap in the list_for_each_entry_safe() block after having store its data pointer and length in local variables. Also interrupt the CRYPTO data process as soon as qc_ssl_provide_quic_data() or qc_treat_rx_crypto_frms() fail. No need to be backported. --- src/quic_ssl.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/quic_ssl.c b/src/quic_ssl.c index 814b1b1c5..bc5f0eacc 100644 --- a/src/quic_ssl.c +++ b/src/quic_ssl.c @@ -638,30 +638,28 @@ int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx) TRACE_ENTER(QUIC_EV_CONN_PHPKTS, qc); list_for_each_entry(qel, &qc->qel_list, list) { - int ssl_ret; struct qf_crypto *qf_crypto, *qf_back; - ssl_ret = 1; list_for_each_entry_safe(qf_crypto, qf_back, &qel->rx.crypto_frms, list) { - ssl_ret = qc_ssl_provide_quic_data(&ncbuf, qel->level, ctx, - qf_crypto->data, qf_crypto->len); + const unsigned char *crypto_data = qf_crypto->data; + size_t crypto_len = qf_crypto->len; + /* Free this frame asap */ LIST_DELETE(&qf_crypto->list); pool_free(pool_head_qf_crypto, qf_crypto); - if (!ssl_ret) { - TRACE_DEVEL("null ssl_ret", QUIC_EV_CONN_PHPKTS, qc, qel); - break; - } + if (!qc_ssl_provide_quic_data(&ncbuf, qel->level, ctx, + crypto_data, crypto_len)) + goto leave; TRACE_DEVEL("buffered crypto data were provided to TLS stack", QUIC_EV_CONN_PHPKTS, qc, qel); } - if (!qc_treat_rx_crypto_frms(qc, qel, ctx)) - ssl_ret = 0; + if (!qel->cstream) + continue; - if (!ssl_ret) + if (!qc_treat_rx_crypto_frms(qc, qel, ctx)) goto leave; }