From: Willy Tarreau <w@1wt.eu>
Date: Mon, 12 May 2025 15:45:33 +0000 (+0200)
Subject: BUG/MEDIUM: h2/h3: reject some forbidden chars in :authority before reassembly
X-Git-Tag: v3.2-dev16~35
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a05c1f57490ba3adb378ad8e6e26830425514e7;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: h2/h3: reject some forbidden chars in :authority before reassembly

As discussed here:
   https://github.com/httpwg/http2-spec/pull/936
   https://github.com/haproxy/haproxy/issues/2941

It's important to take care of some special characters in the :authority
pseudo header before reassembling a complete URI, because after assembly
it's too late (e.g. the '/'). This patch does this, both for h2 and h3.

The impact on H2 was measured in the worst case at 0.3% of the request
rate, while the impact on H3 is around 1%, but H3 was about 1% faster
than H2 before and is now on par.

It may be backported after a period of observation, and in this case it
relies on this previous commit:

   MINOR: http: add a function to validate characters of :authority

Thanks to @DemiMarie for reviving this topic in issue #2941 and bringing
new potential interesting cases.
---

diff --git a/src/h2.c b/src/h2.c
index 3cb0af757..5a8c25068 100644
--- a/src/h2.c
+++ b/src/h2.c
@@ -203,6 +203,17 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
 		}
 	}
 
+	/* We're going to concatenate :authority with :path to form a URI. Some
+	 * characters must absolutely be avoided in :authority to make sure not
+	 * to result in a broken concatenation. See the following links for a
+	 * discussion on this topic:
+	 *   https://github.com/httpwg/http2-spec/pull/936
+	 *   https://github.com/haproxy/haproxy/issues/2941
+	 */
+	if ((fields & H2_PHDR_FND_AUTH) &&
+	    http_authority_has_forbidden_char(phdr[H2_PHDR_IDX_AUTH]))
+		goto fail;
+
 	if (!(flags & HTX_SL_F_HAS_SCHM)) {
 		/* no scheme, use authority only (CONNECT) */
 		uri = phdr[H2_PHDR_IDX_AUTH];
diff --git a/src/h3.c b/src/h3.c
index eb1ff3575..680589a65 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -753,6 +753,21 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
 		goto out;
 	}
 
+	/* We're going to concatenate :authority with :path to form a URI. Some
+	 * characters must absolutely be avoided in :authority to make sure not
+	 * to result in a broken concatenation. See the following links for a
+	 * discussion on this topic:
+	 *   https://github.com/httpwg/http2-spec/pull/936
+	 *   https://github.com/haproxy/haproxy/issues/2941
+	 */
+	if (http_authority_has_forbidden_char(authority)) {
+		TRACE_ERROR("invalid character in authority", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+		h3s->err = H3_ERR_MESSAGE_ERROR;
+		qcc_report_glitch(h3c->qcc, 1);
+		len = -1;
+		goto out;
+	}
+
 	if (!istlen(scheme)) {
 		/* No scheme (CONNECT), use :authority only. */
 		uri = authority;