From c499cd15c71cf3bac7edbb62fc9d60ff401cc5d2 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Fri, 5 Apr 2024 23:54:17 +0200
Subject: [PATCH] BUG/MEDIUM: quic: don't blindly rely on unaligned accesses

There are several places where the QUIC low-level code performs unaligned
accesses by casting unaligned char* pointers to uint32_t, but this is
totally forbidden as it only works on machines that support unaligned
accesses, and either crashes on other ones (SPARC, MIPS), can result in
reading garbage (ARMv5) or be very slow due to the access being emulated
(RISC-V). We do have functions for this, such as read_u32() and write_u32()
that rely on the compiler's knowledge of the machine's capabilities to
either perform an unaligned access or do it one byte at a time.

This must be backported at least as far as 2.6. Some of the code moved a
few times since, so in order to figure the points that need to be fixed,
one may look for a forced pointer cast without having verified that either
the machine is compatible or that the pointer is aligned using this:

  $ git grep 'uint[36][24]_t \*)'

Or build and run the code on a MIPS or SPARC and perform requests using
curl to see if they work or crash with a bus error. All the places fixed
in this commit were found thanks to an immediate crash on the first
request.

This was tagged medium because the affected archs are not the most common
ones where QUIC will be found these days.
---
 src/quic_retry.c |  2 +-
 src/quic_rx.c    |  2 +-
 src/quic_tp.c    | 16 ++++++++--------
 src/quic_tx.c    |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/quic_retry.c b/src/quic_retry.c
index 1c58e5e83..f1d55b878 100644
--- a/src/quic_retry.c
+++ b/src/quic_retry.c
@@ -60,7 +60,7 @@ static int quic_generate_retry_token_aad(unsigned char *aad,
 	unsigned char *p;
 
 	p = aad;
-	*(uint32_t *)p = htonl(version);
+	write_u32(p, htonl(version));
 	p += sizeof version;
 	p += quic_saddr_cpy(p, addr);
 	memcpy(p, cid->data, cid->len);
diff --git a/src/quic_rx.c b/src/quic_rx.c
index 8612c3f00..3645f1673 100644
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1462,7 +1462,7 @@ static inline int quic_read_uint32(uint32_t *val,
 	if (end - *buf < sizeof *val)
 		return 0;
 
-	*val = ntohl(*(uint32_t *)*buf);
+	*val = ntohl(read_u32(*buf));
 	*buf += sizeof *val;
 
 	return 1;
diff --git a/src/quic_tp.c b/src/quic_tp.c
index d13401478..08d24b28f 100644
--- a/src/quic_tp.c
+++ b/src/quic_tp.c
@@ -171,23 +171,23 @@ 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, *others;
+	const unsigned char *ver, *others;
 
 	/* <tp_len> must be a multiple of sizeof(uint32_t) */
 	if (tp_len < sizeof tp->chosen || (tp_len & 0x3))
 		return 0;
 
-	tp->chosen = ntohl(*(uint32_t *)*buf);
+	tp->chosen = ntohl(read_u32(*buf));
 	/* Must not be null */
 	if (!tp->chosen)
 		return 0;
 
 	*buf += sizeof tp->chosen;
-	others = (const uint32_t *)*buf;
+	others = *buf;
 
 	/* Others versions must not be null */
-	for (ver = others; ver < (const uint32_t *)end; ver++) {
-		if (!*ver)
+	for (ver = others; ver < end; ver += 4) {
+		if (!read_u32(ver))
 			return 0;
 	}
 
@@ -195,19 +195,19 @@ static int quic_transport_param_dec_version_info(struct tp_version_information *
 		/* TODO: not supported */
 		return 0;
 
-	for (ver = others; ver < (const uint32_t *)end; ver++) {
+	for (ver = others; ver < end; ver += 4) {
 		if (!tp->negotiated_version) {
 			int i;
 
 			for (i = 0; i < quic_versions_nb; i++) {
-				if (ntohl(*ver) == quic_versions[i].num) {
+				if (ntohl(read_u32(ver)) == quic_versions[i].num) {
 					tp->negotiated_version = &quic_versions[i];
 					break;
 				}
 			}
 		}
 
-		if (preferred_version && ntohl(*ver) == preferred_version->num) {
+		if (preferred_version && ntohl(read_u32(ver)) == preferred_version->num) {
 			tp->negotiated_version = preferred_version;
 			goto out;
 		}
diff --git a/src/quic_tx.c b/src/quic_tx.c
index f9f021cfc..94646a68d 100644
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -1323,7 +1323,7 @@ static inline int quic_write_uint32(unsigned char **buf,
 	if (end - *buf < sizeof val)
 		return 0;
 
-	*(uint32_t *)*buf = htonl(val);
+	write_u32(*buf, htonl(val));
 	*buf += sizeof val;
 
 	return 1;