Reject special characters in URI path component

Resolves: RHEL-18169
2024-01-23 13:35:12 -06:00 · 2024-01-23 13:35:12 -06:00 · 297c7feff7
commit 297c7feff7
parent b329ccf9a7
3 changed files with 198 additions and 1 deletions
--- a/RHEL-18169_h1-reject-chars-uri-path-component.patch
+++ b/RHEL-18169_h1-reject-chars-uri-path-component.patch
@ -0,0 +1,119 @@
+From e5a741f94977840c58775b38f8ed830207f7e4d0 Mon Sep 17 00:00:00 2001
+From: Willy Tarreau <w@1wt.eu>
+Date: Tue, 8 Aug 2023 16:17:22 +0200
+Subject: [PATCH] BUG/MINOR: h1: do not accept '#' as part of the URI component
+
+Seth Manesse and Paul Plasil reported that the "path" sample fetch
+function incorrectly accepts '#' as part of the path component. This
+can in some cases lead to misrouted requests for rules that would apply
+on the suffix:
+
+    use_backend static if { path_end .png .jpg .gif .css .js }
+
+Note that this behavior can be selectively configured using
+"normalize-uri fragment-encode" and "normalize-uri fragment-strip".
+
+The problem is that while the RFC says that this '#' must never be
+emitted, as often it doesn't suggest how servers should handle it. A
+diminishing number of servers still do accept it and trim it silently,
+while others are rejecting it, as indicated in the conversation below
+with other implementers:
+
+   https://lists.w3.org/Archives/Public/ietf-http-wg/2023JulSep/0070.html
+
+Looking at logs from publicly exposed servers, such requests appear at
+a rate of roughly 1 per million and only come from attacks or poorly
+written web crawlers incorrectly following links found on various pages.
+
+Thus it looks like the best solution to this problem is to simply reject
+such ambiguous requests by default, and include this in the list of
+controls that can be disabled using "option accept-invalid-http-request".
+
+We're already rejecting URIs containing any control char anyway, so we
+should also reject '#'.
+
+In the H1 parser for the H1_MSG_RQURI state, there is an accelerated
+parser for bytes 0x21..0x7e that has been tightened to 0x24..0x7e (it
+should not impact perf since 0x21..0x23 are not supposed to appear in
+a URI anyway). This way '#' falls through the fine-grained filter and
+we can add the special case for it also conditionned by a check on the
+proxy's option "accept-invalid-http-request", with no overhead for the
+vast majority of valid URIs. Here this information is available through
+h1m->err_pos that's set to -2 when the option is here (so we don't need
+to change the API to expose the proxy). Example with a trivial GET
+through netcat:
+
+  [08/Aug/2023:16:16:52.651] frontend layer1 (#2): invalid request
+    backend <NONE> (#-1), server <NONE> (#-1), event #0, src 127.0.0.1:50812
+    buffer starts at 0 (including 0 out), 16361 free,
+    len 23, wraps at 16336, error at position 7
+    H1 connection flags 0x00000000, H1 stream flags 0x00000810
+    H1 msg state MSG_RQURI(4), H1 msg flags 0x00001400
+    H1 chunk len 0 bytes, H1 body len 0 bytes :
+
+    00000  GET /aa#bb HTTP/1.0\r\n
+    00021  \r\n
+
+This should be progressively backported to all stable versions along with
+the following patch:
+
+    REGTESTS: http-rules: add accept-invalid-http-request for normalize-uri tests
+
+Similar fixes for h2 and h3 will come in followup patches.
+
+Thanks to Seth Manesse and Paul Plasil for reporting this problem with
+detailed explanations.
+
+(cherry picked from commit 2eab6d354322932cfec2ed54de261e4347eca9a6)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit 9bf75c8e22a8f2537f27c557854a8803087046d0)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit 9facd01c9ac85fe9bcb331594b80fa08e7406552)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit 832b672eee54866c7a42a1d46078cc9ae0d544d9)
+Signed-off-by: Willy Tarreau <w@1wt.eu>
+---
+ src/h1.c | 15 +++++++++++----
+ 1 file changed, 11 insertions(+), 4 deletions(-)
+
+diff --git a/src/h1.c b/src/h1.c
+index eeda311b7..91d3dc47a 100644
+--- a/src/h1.c
+++ b/src/h1.c
+@@ -480,13 +480,13 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
+ 	case H1_MSG_RQURI:
+ 	http_msg_rquri:
+ #ifdef HA_UNALIGNED_LE
+-		/* speedup: skip bytes not between 0x21 and 0x7e inclusive */
+		/* speedup: skip bytes not between 0x24 and 0x7e inclusive */
+ 		while (ptr <= end - sizeof(int)) {
+-			int x = *(int *)ptr - 0x21212121;
+			int x = *(int *)ptr - 0x24242424;
+ 			if (x & 0x80808080)
+ 				break;
+ 
+-			x -= 0x5e5e5e5e;
+			x -= 0x5b5b5b5b;
+ 			if (!(x & 0x80808080))
+ 				break;
+ 
+@@ -498,8 +498,15 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
+ 			goto http_msg_ood;
+ 		}
+ 	http_msg_rquri2:
+-		if (likely((unsigned char)(*ptr - 33) <= 93)) /* 33 to 126 included */
+		if (likely((unsigned char)(*ptr - 33) <= 93)) { /* 33 to 126 included */
+			if (*ptr == '#') {
+				if (h1m->err_pos < -1) /* PR_O2_REQBUG_OK not set */
+					goto invalid_char;
+				if (h1m->err_pos == -1) /* PR_O2_REQBUG_OK set: just log */
+					h1m->err_pos = ptr - start + skip;
+			}
+ 			EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_rquri2, http_msg_ood, state, H1_MSG_RQURI);
+		}
+ 
+ 		if (likely(HTTP_IS_SPHT(*ptr))) {
+ 			sl.rq.u.len = ptr - sl.rq.u.ptr;
+-- 
+2.43.0
+
--- a/RHEL-18169_h2-reject-chars-uri-path-component.patch
+++ b/RHEL-18169_h2-reject-chars-uri-path-component.patch
@ -0,0 +1,71 @@
+From af232e47e6264122bed3681210b054ff38ec8de8 Mon Sep 17 00:00:00 2001
+From: Willy Tarreau <w@1wt.eu>
+Date: Tue, 8 Aug 2023 15:40:49 +0200
+Subject: [PATCH] BUG/MINOR: h2: reject more chars from the :path pseudo header
+
+This is the h2 version of this previous fix:
+
+    BUG/MINOR: h1: do not accept '#' as part of the URI component
+
+In addition to the current NUL/CR/LF, this will also reject all other
+control chars, the space and '#' from the :path pseudo-header, to avoid
+taking the '#' for a part of the path. It's still possible to fall back
+to the previous behavior using "option accept-invalid-http-request".
+
+This patch modifies the request parser to change the ":path" pseudo header
+validation function with a new one that rejects 0x00-0x1F (control chars),
+space and '#'. This way such chars will be dropped early in the chain, and
+the search for '#' doesn't incur a second pass over the header's value.
+
+This should be progressively backported to stable versions, along with the
+following commits it relies on:
+
+     REGTESTS: http-rules: add accept-invalid-http-request for normalize-uri tests
+     REORG: http: move has_forbidden_char() from h2.c to http.h
+     MINOR: ist: add new function ist_find_range() to find a character range
+     MINOR: http: add new function http_path_has_forbidden_char()
+     MINOR: h2: pass accept-invalid-http-request down the request parser
+
+(cherry picked from commit b3119d4fb4588087e2483a80b01d322683719e29)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit 462a8600ce9e478573a957e046b446a7dcffd286)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit 648e59e30723b8fd4e71aab02cb679f6ea7446e7)
+Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
+(cherry picked from commit c8e07f2fd8b5462527f102f7145d6027c0d041da)
+[wt: minor ctx adjustments]
+Signed-off-by: Willy Tarreau <w@1wt.eu>
+---
+ src/h2.c | 15 +++++++++++----
+ 1 file changed, 11 insertions(+), 4 deletions(-)
+
+diff --git a/src/h2.c b/src/h2.c
+index 94c384111..e190c52b5 100644
+--- a/src/h2.c
+++ b/src/h2.c
+@@ -440,11 +440,18 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
+ 		}
+ 
+ 		/* RFC7540#10.3: intermediaries forwarding to HTTP/1 must take care of
+-		 * rejecting NUL, CR and LF characters.
+		 * rejecting NUL, CR and LF characters. For :path we reject all CTL
+		 * chars, spaces, and '#'.
+ 		 */
+-		ctl = ist_find_ctl(list[idx].v);
+-		if (unlikely(ctl) && has_forbidden_char(list[idx].v, ctl))
+-			goto fail;
+		if (phdr == H2_PHDR_IDX_PATH && !relaxed) {
+			ctl = ist_find_range(list[idx].v, 0, '#');
+			if (unlikely(ctl) && http_path_has_forbidden_char(list[idx].v, ctl))
+				goto fail;
+		} else {
+			ctl = ist_find_ctl(list[idx].v);
+			if (unlikely(ctl) && has_forbidden_char(list[idx].v, ctl))
+				goto fail;
+		}
+ 
+ 		if (phdr > 0 && phdr < H2_PHDR_NUM_ENTRIES) {
+ 			/* insert a pseudo header by its index (in phdr) and value (in value) */
+-- 
+2.43.0
+
--- a/haproxy.spec
+++ b/haproxy.spec
@ -8,7 +8,7 @@

 Name:           haproxy
 Version:        2.4.22
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        HAProxy reverse proxy for high availability environments

 License:        GPLv2+
@ -23,6 +23,8 @@ Source5:        %{name}.sysusers
 Source6:        halog.1

 Patch0:	RHEL-7736_http-reject-empty-content-length-header.patch
+Patch1: RHEL-18169_h1-reject-chars-uri-path-component.patch
+Patch2: RHEL-18169_h2-reject-chars-uri-path-component.patch

 BuildRequires:  gcc
 BuildRequires:  lua-devel
@ -53,6 +55,8 @@ availability environments. Indeed, it can:
 %prep
 %setup -q
 %patch -P0 -p1
+%patch -P1 -p1
+%patch -P2 -p1

 %build
 regparm_opts=
@ -135,6 +139,9 @@ done
 %{_sysusersdir}/%{name}.conf

 %changelog
+* Tue Jan 23 2024 Ryan O'Hara <rohara@redhat.com> - 2.4.22-3
+- Reject "#" as part of USI component (CVE-2023-45539, RHEL-18169)
+
 * Wed Jan 17 2024 Ryan O'Hara <rohara@redhat.com> - 2.4.22-2
 - Reject any empty content-length header value (CVE-2023-40225, RHEL-7736)