From: Willy Tarreau <w@1wt.eu>
Date: Fri, 16 May 2025 12:58:52 +0000 (+0200)
Subject: BUG/MEDIUM: h1/h2/h3: reject forbidden chars in the Host header field
X-Git-Tag: v3.2-dev17~39
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=df00164fdd98d15e832daad34fb23249083bfb9c;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: h1/h2/h3: reject forbidden chars in the Host header field

In continuation with 9a05c1f574 ("BUG/MEDIUM: h2/h3: reject some
forbidden chars in :authority before reassembly") and the discussion
in issue #2941, @DemiMarie rightfully suggested that Host should also
be sanitized, because it is sometimes used in concatenation, such as
this:

    http-request set-url https://%[req.hdr(host)]%[pathq]

which was proposed as a workaround for h2 upstream servers that require
:authority here:

    https://www.mail-archive.com/haproxy@formilux.org/msg43261.html

The current patch then adds the same check for forbidden chars in the
Host header, using the same function as for the patch above, since in
both cases we validate the host:port part of the authority. This way
we won't reconstruct ambiguous URIs by concatenating Host and path.

Just like the patch above, this can be backported afer a period of
observation.
---

diff --git a/src/h1.c b/src/h1.c
index 5708496ba..1ad5ac19f 100644
--- a/src/h1.c
+++ b/src/h1.c
@@ -986,8 +986,14 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 					h1_parse_upgrade_header(h1m, v);
 				}
 				else if (!(h1m->flags & H1_MF_RESP) && isteqi(n, ist("host"))) {
-					if (host_idx == -1)
+					if (host_idx == -1) {
 						host_idx = hdr_count;
+						if (http_authority_has_forbidden_char(v)) {
+							state = H1_MSG_HDR_L2_LWS;
+							ptr = v.ptr; /* Set ptr on the error */
+							goto http_msg_invalid;
+						}
+					}
 					else {
 						if (!isteqi(v, hdr[host_idx].v)) {
 							state = H1_MSG_HDR_L2_LWS;
diff --git a/src/h2.c b/src/h2.c
index 5a8c25068..491f62323 100644
--- a/src/h2.c
+++ b/src/h2.c
@@ -411,10 +411,13 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
 		}
 
 		if (isteq(list[idx].n, ist("host"))) {
+			/* skip duplicates */
 			if (fields & H2_PHDR_FND_HOST)
 				continue;
 
 			fields |= H2_PHDR_FND_HOST;
+			if (http_authority_has_forbidden_char(list[idx].v))
+				goto fail;
 		}
 
 		if (isteq(list[idx].n, ist("content-length"))) {
diff --git a/src/h3.c b/src/h3.c
index d03c963c5..b2f6750fb 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -863,7 +863,8 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
 		if (isteq(list[hdr_idx].n, ist("host"))) {
 			struct ist prev_auth = authority;
 
-			if (h3_set_authority(qcs, &authority, list[hdr_idx].v)) {
+			if (http_authority_has_forbidden_char(list[hdr_idx].v) ||
+			    h3_set_authority(qcs, &authority, list[hdr_idx].v)) {
 				h3s->err = H3_ERR_MESSAGE_ERROR;
 				qcc_report_glitch(h3c->qcc, 1);
 				len = -1;