From 1d00d701b1619b4be0f82a0d4f3ac11d21aea1c2 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Fri, 21 Nov 2025 09:52:41 +0100 Subject: [PATCH] BUG/MEDIUM: quic-be: quic_conn_closed buffer overflow This bug impacts only the backends. Recent commits have modified quic_rx_pkt_parse() for the QUIC backend to handle the retry token, and version negotiation. This function is called for the quic_conn even when is closing state (so for the quic_conn_closed struct). The quic_conn struct and quic_conn_closed struct share some members thank to the leading QUIC_CONN_COMMON struct. The recent modification impacts some members which do not exist for the quic_connn_closed struct, leading to buffer overflows if modified. For the backends only this patch: 1- silently drops the Retry packet (received/parsed only by backends) 2- silently drops the Initial packets received in closing state This is safe for the Initial packets because in closing state the datagrams are entirely skipped thanks to qc_rx_check_closing() in quic_dgram_parse(). No backport needed because the backend support arrived with the current dev. --- src/quic_rx.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/quic_rx.c b/src/quic_rx.c index f4e3f7d6b..ca4f916f6 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -1982,6 +1982,12 @@ static int quic_rx_pkt_parse(struct quic_conn *qc, struct quic_rx_packet *pkt, pos += pkt->token_len; } else if (pkt->type == QUIC_PACKET_TYPE_RETRY) { + if (qc->flags & QUIC_FL_CONN_CLOSING) { + TRACE_PROTO("Packet dropped", + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); + goto drop; + } + if (!quic_retry_packet_check(qc, pkt, beg, end, pos, &qc->retry_token_len)) /* TODO: should close the connection? */ goto drop; @@ -2079,26 +2085,34 @@ static int quic_rx_pkt_parse(struct quic_conn *qc, struct quic_rx_packet *pkt, qc->dcid.len = pkt->scid.len; } - /* Identify the negotiated version, chosen and sent by the server */ - if (qc_is_back(qc) && pkt->version != qc->original_version && !qc->nictx) { - qc->nictx = pool_alloc(pool_head_quic_tls_ctx); - if (!qc->nictx) { - TRACE_PROTO("Could not alloc a new Initial secrets TLS context", - QUIC_EV_CONN_RXPKT, qc); + if (qc_is_back(qc)) { + if (qc->flags & QUIC_FL_CONN_CLOSING) { + TRACE_PROTO("Packet dropped", + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); goto drop; } - quic_tls_ctx_reset(qc->nictx); - if (!qc_new_isecs(qc, qc->nictx, pkt->version, - qc->odcid.data, qc->odcid.len, 0)) { - TRACE_PROTO("Could not derive Initial secrets for new version", - QUIC_EV_CONN_RXPKT, qc); - goto drop; - } + /* Identify the negotiated version, chosen and sent by the server */ + if (pkt->version != qc->original_version && !qc->nictx) { + qc->nictx = pool_alloc(pool_head_quic_tls_ctx); + if (!qc->nictx) { + TRACE_PROTO("Could not alloc a new Initial secrets TLS context", + QUIC_EV_CONN_RXPKT, qc); + goto drop; + } - TRACE_PROTO("new Initial secrets TLS context initialization done", - QUIC_EV_CONN_RXPKT, qc); - qc->negotiated_version = pkt->version; + quic_tls_ctx_reset(qc->nictx); + if (!qc_new_isecs(qc, qc->nictx, pkt->version, + qc->odcid.data, qc->odcid.len, 0)) { + TRACE_PROTO("Could not derive Initial secrets for new version", + QUIC_EV_CONN_RXPKT, qc); + goto drop; + } + + TRACE_PROTO("new Initial secrets TLS context initialization done", + QUIC_EV_CONN_RXPKT, qc); + qc->negotiated_version = pkt->version; + } } } }