From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Wed, 10 May 2023 11:06:31 +0000 (+0200)
Subject: BUG/MINOR: quic: Possible crash when dumping version information
X-Git-Tag: v2.8-dev11~8
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b97169629696dd7cc4c8a3f15aa0c960ee113272;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/include/haproxy/quic_tp-t.h b/include/haproxy/quic_tp-t.h
index 29f993b41e..8ca69c7319 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 c5959dfffe..af822641a3 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 09921f360f..9879828417 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;
 
 	/* <tp_len> 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;