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.
This commit is contained in:
Frederic Lecaille 2025-11-21 09:52:41 +01:00
parent 0629ce8f4b
commit 1d00d701b1

View File

@ -1982,6 +1982,12 @@ static int quic_rx_pkt_parse(struct quic_conn *qc, struct quic_rx_packet *pkt,
pos += pkt->token_len; pos += pkt->token_len;
} }
else if (pkt->type == QUIC_PACKET_TYPE_RETRY) { 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)) if (!quic_retry_packet_check(qc, pkt, beg, end, pos, &qc->retry_token_len))
/* TODO: should close the connection? */ /* TODO: should close the connection? */
goto drop; 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; qc->dcid.len = pkt->scid.len;
} }
/* Identify the negotiated version, chosen and sent by the server */ if (qc_is_back(qc)) {
if (qc_is_back(qc) && pkt->version != qc->original_version && !qc->nictx) { if (qc->flags & QUIC_FL_CONN_CLOSING) {
qc->nictx = pool_alloc(pool_head_quic_tls_ctx); TRACE_PROTO("Packet dropped",
if (!qc->nictx) { QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
TRACE_PROTO("Could not alloc a new Initial secrets TLS context",
QUIC_EV_CONN_RXPKT, qc);
goto drop; goto drop;
} }
quic_tls_ctx_reset(qc->nictx); /* Identify the negotiated version, chosen and sent by the server */
if (!qc_new_isecs(qc, qc->nictx, pkt->version, if (pkt->version != qc->original_version && !qc->nictx) {
qc->odcid.data, qc->odcid.len, 0)) { qc->nictx = pool_alloc(pool_head_quic_tls_ctx);
TRACE_PROTO("Could not derive Initial secrets for new version", if (!qc->nictx) {
QUIC_EV_CONN_RXPKT, qc); TRACE_PROTO("Could not alloc a new Initial secrets TLS context",
goto drop; QUIC_EV_CONN_RXPKT, qc);
} goto drop;
}
TRACE_PROTO("new Initial secrets TLS context initialization done", quic_tls_ctx_reset(qc->nictx);
QUIC_EV_CONN_RXPKT, qc); if (!qc_new_isecs(qc, qc->nictx, pkt->version,
qc->negotiated_version = 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;
}
} }
} }
} }