From 89e48ff92f3ac7645a4d90fc352b88aba8af802a Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 19 Apr 2023 10:04:41 +0200 Subject: [PATCH] BUG/MEDIUM: quic: prevent crash on Retry sending The following commit introduced a regression : commit 1a5cc19cecfa84bfd0fdd7b9d5d20899cce40662 MINOR: quic: adjust Rx packet type parsing Since this commit, qv variable was left to NULL as version is stored directly in quic_rx_packet instance. In most cases, this only causes traces to skip version printing. However, qv is dereferenced when sending a Retry which causes a segfault. To fix this, simply remove qv variable and use pkt->version instead, both for traces and send_retry() invocation. This bug was detected thanks to QUIC interop runner. It can easily be reproduced by using quic-force-retry on the bind line. This must be backported up to 2.7. --- src/quic_conn.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/quic_conn.c b/src/quic_conn.c index b6f146775..02e4044a4 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -6867,7 +6867,6 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, const unsigned char *beg = buf; struct proxy *prx; struct quic_counters *prx_counters; - const struct quic_version *qv = NULL; TRACE_ENTER(QUIC_EV_CONN_LPKT); @@ -6952,7 +6951,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, if (!quic_dec_int(&token_len, (const unsigned char **)&buf, end) || end - buf < token_len) { TRACE_PROTO("Packet dropped", - QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv); + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); goto drop; } @@ -6962,10 +6961,10 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, if (global.cluster_secret && !token_len) { if (l->bind_conf->options & BC_O_QUIC_FORCE_RETRY) { TRACE_PROTO("Initial without token, sending retry", - QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv); - if (send_retry(l->rx.fd, &dgram->saddr, pkt, qv)) { + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); + if (send_retry(l->rx.fd, &dgram->saddr, pkt, pkt->version)) { TRACE_PROTO("Error during Retry generation", - QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv); + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); goto drop_silent; } @@ -6978,7 +6977,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, * cluster secret. */ TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, - NULL, NULL, NULL, qv); + NULL, NULL, NULL, pkt->version); goto drop; } @@ -6989,7 +6988,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, else if (pkt->type != QUIC_PACKET_TYPE_0RTT) { if (pkt->dcid.len != QUIC_HAP_CID_LEN) { TRACE_PROTO("Packet dropped", - QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv); + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); goto drop; } } @@ -6997,7 +6996,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, if (!quic_dec_int(&len, (const unsigned char **)&buf, end) || end - buf < len) { TRACE_PROTO("Packet dropped", - QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qv); + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); goto drop; } @@ -7056,7 +7055,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, pkt->len = end - beg; } - TRACE_PROTO("RX pkt parsed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, qv); + TRACE_PROTO("RX pkt parsed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, pkt->version); TRACE_LEAVE(QUIC_EV_CONN_LPKT); return 0; @@ -7065,7 +7064,7 @@ static int quic_rx_pkt_parse(struct quic_rx_packet *pkt, drop_silent: if (!pkt->len) pkt->len = end - beg; - TRACE_PROTO("RX pkt parsing failed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, qv); + TRACE_PROTO("RX pkt parsing failed", QUIC_EV_CONN_LPKT, NULL, pkt, NULL, pkt->version); TRACE_LEAVE(QUIC_EV_CONN_LPKT); return -1; }