Compare commits

...

2 Commits

Author SHA1 Message Date
Ryan O'Hara c84d660d62 Reject special characters in URI path component Resolves: RHEL-18169 2024-01-24 02:08:14 +00:00
Ryan O'Hara b329ccf9a7 Fix patch syntax
Related: RHEL-7736
2024-01-22 12:19:15 -06:00
4 changed files with 200 additions and 2 deletions

1
.haproxy.metadata Normal file
View File

@ -0,0 +1 @@
d0654cbab48039d998fca2459ce9251c6dbf2ae8 haproxy-2.4.22.tar.gz

View File

@ -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

View File

@ -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

View File

@ -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
@ -52,7 +54,9 @@ availability environments. Indeed, it can:
%prep
%setup -q
%patch 0 -p1
%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)