From: Gihyeon Ryu Date: Wed, 11 Nov 2026 10:00:11 +0000 (+0000) Subject: BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame X-Git-Tag: v3.5-dev12~38 X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=827354038df8b6fb4603116ecc85ec2d54478cd5;p=haproxy.git BUG/MAJOR: quic: don't reference a retired CID from a NEW_CONNECTION_ID frame A NEW_CONNECTION_ID frame built by HAProxy only stores pointers to the CID and stateless reset token of its quic_connection_id. When a RETIRE_CONNECTION_ID frame is received, the CID is freed at once, while a NEW_CONNECTION_ID frame for it may still be queued, in flight, or be duplicated later for a retransmission. A client can trigger it by retiring sequence numbers 1 and 2 in the same packet: the second frame frees the CID allocated by the first one, whose frame is not sent yet. The frame is then built from freed memory. In a default build, the pool cache gives the object back to the next CID of the same connection, so two frames carry the same CID. Without the pool cache, freed heap memory is sent to the peer, and ASan reports a heap-use-after-free. struct qf_new_connection_id now stores the CID and the token instead of pointers, and quic_connection_id_to_frm_cpy() and the frame duplication copy them. The parser copies them as well and rejects a CID length above 20 (RFC 9000 section 19.15). quic_frame does not grow, since its union already holds a larger member. This should be backported to 2.8 and later. --- diff --git a/include/haproxy/quic_cid.h b/include/haproxy/quic_cid.h index 05f3e5b80..3aa7c481b 100644 --- a/include/haproxy/quic_cid.h +++ b/include/haproxy/quic_cid.h @@ -111,8 +111,9 @@ static inline void quic_connection_id_to_frm_cpy(struct quic_frame *dst, ncid_frm->seq_num = src->seq_num.key; ncid_frm->retire_prior_to = src->retire_prior_to; ncid_frm->cid.len = src->cid.len; - ncid_frm->cid.data = src->cid.data; - ncid_frm->stateless_reset_token = src->stateless_reset_token; + memcpy(ncid_frm->cid.data, src->cid.data, src->cid.len); + memcpy(ncid_frm->stateless_reset_token, src->stateless_reset_token, + QUIC_STATELESS_RESET_TOKEN_LEN); } #endif /* USE_QUIC */ diff --git a/include/haproxy/quic_frame-t.h b/include/haproxy/quic_frame-t.h index c94aff0f5..7db31c9c2 100644 --- a/include/haproxy/quic_frame-t.h +++ b/include/haproxy/quic_frame-t.h @@ -32,6 +32,8 @@ #include #include #include +#include +#include #include extern struct pool_head *pool_head_quic_frame; @@ -217,14 +219,18 @@ struct qf_streams_blocked { uint64_t limit; }; +/* The CID and the token are stored in the frame, because a frame which is + * waiting to be sent or retransmitted must not depend on the lifetime of the + * quic_connection_id it was built from: the peer may retire it at any time. + */ struct qf_new_connection_id { uint64_t seq_num; uint64_t retire_prior_to; struct { unsigned char len; - const unsigned char *data; + unsigned char data[QUIC_CID_MAXLEN]; } cid; - const unsigned char *stateless_reset_token; + unsigned char stateless_reset_token[QUIC_STATELESS_RESET_TOKEN_LEN]; }; struct qf_retire_connection_id { diff --git a/src/quic_frame.c b/src/quic_frame.c index 1e813ee7f..0233869bd 100644 --- a/src/quic_frame.c +++ b/src/quic_frame.c @@ -807,14 +807,19 @@ static int quic_parse_new_connection_id_frame(struct quic_frame *frm, struct qui return 0; ncid_frm->cid.len = *(*pos)++; - if (end - *pos < ncid_frm->cid.len + QUIC_STATELESS_RESET_TOKEN_LEN) + /* RFC 9000 19.15. NEW_CONNECTION_ID Frames: + * Values less than 1 and greater than 20 are invalid and MUST be + * treated as a connection error of type FRAME_ENCODING_ERROR. + */ + if (ncid_frm->cid.len > QUIC_CID_MAXLEN || + end - *pos < ncid_frm->cid.len + QUIC_STATELESS_RESET_TOKEN_LEN) return 0; if (ncid_frm->cid.len) { - ncid_frm->cid.data = *pos; + memcpy(ncid_frm->cid.data, *pos, ncid_frm->cid.len); *pos += ncid_frm->cid.len; } - ncid_frm->stateless_reset_token = *pos; + memcpy(ncid_frm->stateless_reset_token, *pos, QUIC_STATELESS_RESET_TOKEN_LEN); *pos += QUIC_STATELESS_RESET_TOKEN_LEN; return 1;