From b97169629696dd7cc4c8a3f15aa0c960ee113272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Wed, 10 May 2023 13:06:31 +0200 Subject: [PATCH] BUG/MINOR: quic: Possible crash when dumping version information ->others member of tp_version_information structure pointed to a buffer in the TLS stack used to parse the transport parameters. There is no garantee that this buffer is available until the connection is released. Do not dump the available versions selected by the client anymore, but displayed the chosen one (selected by the client for this connection) and the negotiated one. Must be backported to 2.7 and 2.6. --- include/haproxy/quic_tp-t.h | 2 -- include/haproxy/quic_tp.h | 18 +++--------------- src/quic_tp.c | 9 ++++----- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/include/haproxy/quic_tp-t.h b/include/haproxy/quic_tp-t.h index 29f993b41..8ca69c731 100644 --- a/include/haproxy/quic_tp-t.h +++ b/include/haproxy/quic_tp-t.h @@ -28,8 +28,6 @@ struct tp_preferred_address { struct tp_version_information { uint32_t chosen; - const uint32_t *others; - size_t nb_others; const struct quic_version *negotiated_version; }; diff --git a/include/haproxy/quic_tp.h b/include/haproxy/quic_tp.h index c5959dfff..af822641a 100644 --- a/include/haproxy/quic_tp.h +++ b/include/haproxy/quic_tp.h @@ -45,21 +45,9 @@ static inline void quic_tp_version_info_dump(struct buffer *b, if (!tp->chosen) return; - chunk_appendf(b, " version_information:(chosen=0x%08x", tp->chosen); - if (tp->nb_others) { - int i = 0; - const uint32_t *ver; - chunk_appendf(b, ",others="); - for (ver = tp->others; i < tp->nb_others; i++, ver++) { - if (i != 0) - chunk_appendf(b, ","); - if (local) - chunk_appendf(b, "0x%08x", *ver); - else - chunk_appendf(b, "0x%08x", ntohl(*ver)); - } - } - chunk_appendf(b, ")"); + chunk_appendf(b, " versions:chosen=0x%08x", tp->chosen); + if (tp->negotiated_version) + chunk_appendf(b, ",negotiated=0x%08x", tp->negotiated_version->num); } static inline void quic_transport_params_dump(struct buffer *b, diff --git a/src/quic_tp.c b/src/quic_tp.c index 09921f360..987982841 100644 --- a/src/quic_tp.c +++ b/src/quic_tp.c @@ -172,7 +172,7 @@ static int quic_transport_param_dec_version_info(struct tp_version_information * const unsigned char *end, int server) { size_t tp_len = end - *buf; - const uint32_t *ver; + const uint32_t *ver, *others; /* must be a multiple of sizeof(uint32_t) */ if (tp_len < sizeof tp->chosen || (tp_len & 0x3)) @@ -184,10 +184,10 @@ static int quic_transport_param_dec_version_info(struct tp_version_information * return 0; *buf += sizeof tp->chosen; - tp->others = (const uint32_t *)*buf; + others = (const uint32_t *)*buf; /* Others versions must not be null */ - for (ver = tp->others; ver < (const uint32_t *)end; ver++) { + for (ver = others; ver < (const uint32_t *)end; ver++) { if (!*ver) return 0; } @@ -196,8 +196,7 @@ static int quic_transport_param_dec_version_info(struct tp_version_information * /* TODO: not supported */ return 0; - tp->nb_others = (end - (const unsigned char *)tp->others) / sizeof *tp->others; - for (ver = tp->others; ver < (const uint32_t *)end; ver++) { + for (ver = others; ver < (const uint32_t *)end; ver++) { if (!tp->negotiated_version) { int i;