From: William Lallemand <wlallemand@haproxy.com>
Date: Thu, 13 Aug 2026 15:52:25 +0000 (+0000)
Subject: BUG/MINOR: payload: fix handshake length off-by-4 in ssl_hello_sni/alpn
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f507ae7530bf43b9c0a5604c3c8d97c4e8f8a9a;p=thirdparty%2Fhaproxy.git

BUG/MINOR: payload: fix handshake length off-by-4 in ssl_hello_sni/alpn

smp_fetch_ssl_hello_sni() and smp_fetch_ssl_hello_alpn() each have
their own copy of the ClientHello preamble parser instead of using the
shared smp_client_hello_parse(). The handshake length ("hs_len") is
compared against "bleft" before "bleft" is decremented for the 4-byte
handshake header (msg_type + 3-byte length), so a ClientHello claiming
a body up to 4 bytes larger than what was actually sent is still
accepted as complete. Every subsequent length-derived bound inherits
this same 4-byte over-count, letting the final read (SNI hostname or
ALPN protocol name) run up to 4 bytes past the received buffer and
disclose uninitialized memory through req.ssl_sni / req.ssl_alpn.

This is the same issue already fixed in smp_client_hello_parse() by
commit 2653936510 ("BUG/MINOR: payload: fix the handshake length
bounds check smp_client_hello_parse()"), just never ported to these
two functions' own duplicated preamble. This applies the same fix:
"data += 4" now comes with "bleft -= 4" before the length check
instead of after it, and the record-layer length is checked as soon
as the handshake is entered.

This must be backported to all supported versions.
---

diff --git a/src/payload.c b/src/payload.c
index 5e39133ba..167fb4303 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -806,6 +806,9 @@ smp_fetch_ssl_hello_sni(const struct arg *args, struct sample *smp, const char *
 	data += 5; /* enter TLS handshake */
 	bleft -= 5;
 
+	if (bleft < hs_len)
+		goto too_short;
+
 	/* Check for a complete client hello starting at <data> */
 	if (bleft < 1)
 		goto too_short;
@@ -819,15 +822,18 @@ smp_fetch_ssl_hello_sni(const struct arg *args, struct sample *smp, const char *
 	if (hs_len < 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
 		goto not_ssl_hello; /* too short to have an extension */
 
+	data += 4;
+	bleft -= 4;
+
 	/* We want the full handshake here */
 	if (bleft < hs_len)
 		goto too_short;
 
-	data += 4;
 	/* Start of the ClientHello message */
 	if (data[0] < 0x03 || data[1] < 0x01) /* TLSv1 minimum */
 		goto not_ssl_hello;
 
+	/* Note: covered by the hs_len test 30 lines above */
 	ext_len = data[34]; /* session_id_len */
 	if (ext_len > 32 || ext_len > (hs_len - 35)) /* check for correct session_id len */
 		goto not_ssl_hello;
@@ -979,6 +985,9 @@ smp_fetch_ssl_hello_alpn(const struct arg *args, struct sample *smp, const char
 	data += 5; /* enter TLS handshake */
 	bleft -= 5;
 
+	if (bleft < hs_len)
+		goto too_short;
+
 	/* Check for a complete client hello starting at <data> */
 	if (bleft < 1)
 		goto too_short;
@@ -992,15 +1001,18 @@ smp_fetch_ssl_hello_alpn(const struct arg *args, struct sample *smp, const char
 	if (hs_len < 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
 		goto not_ssl_hello; /* too short to have an extension */
 
+	data += 4;
+	bleft -= 4;
+
 	/* We want the full handshake here */
 	if (bleft < hs_len)
 		goto too_short;
 
-	data += 4;
 	/* Start of the ClientHello message */
 	if (data[0] < 0x03 || data[1] < 0x01) /* TLSv1 minimum */
 		goto not_ssl_hello;
 
+	/* Note: covered by the hs_len test 30 lines above */
 	ext_len = data[34]; /* session_id_len */
 	if (ext_len > 32 || ext_len > (hs_len - 35)) /* check for correct session_id len */
 		goto not_ssl_hello;