packages/haproxy
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CVE-2024-53008

Browse Source 
(cherry picked from commit c8f478dd041baad66798d303f76d5546a312ed7d)
This commit is contained in:
wk333 2024-12-10 11:37:51 +08:00 committed by openeuler-sync-bot
parent 92cae65eb5
commit 8279e567c8
2 changed files with 523 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

518
CVE-2024-53008.patch Normal file
View File

@ -0,0 +1,518 @@
Orgin:
pre patch:
https://github.com/haproxy/haproxy/commit/1e4d26aee5e9c846c63b6f2955966826b7a488d0
https://github.com/haproxy/haproxy/commit/dec64351957e10cbcd67d9bd1755a406c1dd9eff
https://github.com/haproxy/haproxy/commit/434f74c8128337146f6828da731b9c9d4fb0499e
fix patch:
https://github.com/haproxy/haproxy/commit/fa8b221756076186315b6bbf17ef697ec1ef5695
https://github.com/haproxy/haproxy/commit/94d74d24ec9c3710334ab2239b1996faab3ad01e
From 1e4d26aee5e9c846c63b6f2955966826b7a488d0 Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Wed, 7 Dec 2022 14:33:26 +0100
Subject: [PATCH] BUG/MEDIUM: h3: reject request with invalid pseudo header
RFC 9114 dictates several requirements for pseudo header usage in H3
request. Previously only minimal checks were implemented. Enforce all
the following requirements with this patch :
* reject request with undefined or invalid pseudo header
* reject request with duplicated pseudo header
* reject non-CONNECT request with missing mandatory pseudo header
* reject request with pseudo header after standard ones
For the moment, this kind of errors triggers a connection close. In the
future, it should be handled only with a stream reset. To reduce
backport surface, this will be implemented in another commit.
This must be backported up to 2.6.
From dec64351957e10cbcd67d9bd1755a406c1dd9eff Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 8 Dec 2022 16:54:42 +0100
Subject: [PATCH] BUG/MEDIUM: h3: parse content-length and reject invalid
messages
Ensure that if a request contains a content-length header it matches
with the total size of following DATA frames. This is conformance with
HTTP/3 RFC 9114.
For the moment, this kind of errors triggers a connection close. In the
future, it should be handled only with a stream reset. To reduce
backport surface, this will be implemented in another commit.
This must be backported up to 2.6. It relies on the previous commit :
MINOR: http: extract content-length parsing from H2
From 434f74c8128337146f6828da731b9c9d4fb0499e Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 15 Dec 2022 10:53:55 +0100
Subject: [PATCH] BUG/MINOR: h3: fix memleak on HEADERS parsing failure
If an error is triggered on H3 HEADERS parsing, the allocated buffer for
HTX data is not freed.
To prevent this memleak, all return path have been centralized using
goto statements.
Also, as a small bonus, offer_buffers() is not called anymore if buffer
is not freed because sedesc has taken it. However this change has
probably no noticeable effect as dynamic buffers management is not
functional currently.
This should be backported up to 2.6.
From fa8b221756076186315b6bbf17ef697ec1ef5695 Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Fri, 28 Jun 2024 10:43:19 +0200
Subject: [PATCH] BUG/MEDIUM: h3: ensure the ":method" pseudo header is totally
valid
Ensure pseudo-header method is only constitued of valid characters
according to RFC 9110. If an invalid value is found, the request is
rejected and stream is resetted.
Previously only characters forbidden in headers were rejected (NUL/CR/LF),
but this is insufficient for :method, where some other forbidden chars
might be used to trick a non-compliant backend server into seeing a
different path from the one seen by haproxy. Note that header injection
is not possible though.
This must be backported up to 2.6.
Many thanks to Yuki Mogi of FFRI Security Inc for the detailed report
that allowed to quicky spot, confirm and fix the problem.
(cherry picked from commit 789d4abd7328f0a745d67698e89bbb888d4d9b2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 47d13c68cf198467a94e85a1caa44484a1e2e75c)
[cf: adapted]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 87fefebfbe3df218103502046a0871b235a48087)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 94d305eaffc83dff3f59f5c2a3fbeb4710efa39a)
[cf: There is no err field in h3s]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
From 94d74d24ec9c3710334ab2239b1996faab3ad01e Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Fri, 28 Jun 2024 10:50:19 +0200
Subject: [PATCH] BUG/MEDIUM: h3: ensure the ":scheme" pseudo header is totally
valid
Ensure pseudo-header scheme is only constitued of valid characters
according to RFC 9110. If an invalid value is found, the request is
rejected and stream is resetted.
It's the same as for previous commit "BUG/MEDIUM: h3: ensure the
":method" pseudo header is totally valid" except that this time it
applies to the ":scheme" pseudo header.
This must be backported up to 2.6.
(cherry picked from commit a3bed52d1f84ba36af66be4317a5f746d498bdf4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 5ddc4004cb0c3c4ea4f4596577c85f004678e9c0)
[cf: adapted]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 6748a47819c263d4631187b6f121b5344ab50d57)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 56ab17d34a32d9c15558c2c2d17b743e6d679cbd)
[cf: There is no err field in h3s]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
---
include/haproxy/http.h | 14 +++
src/h3.c | 225 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 212 insertions(+), 27 deletions(-)
diff --git a/include/haproxy/http.h b/include/haproxy/http.h
index ee2f922..711883f 100644
--- a/include/haproxy/http.h
+++ b/include/haproxy/http.h
@@ -171,6 +171,20 @@ static inline struct http_uri_parser http_uri_parser_init(const struct ist uri)
return parser;
}
+/* Check that method only contains token as required.
+ * See RFC 9110 9. Methods
+ */
+static inline int http_method_has_forbidden_char(const struct ist ist)
+{
+ const char *start = istptr(ist);
+ do {
+ if (!HTTP_IS_TOKEN(*start))
+ return 1;
+ start++;
+ } while (start < istend(ist));
+ return 0;
+}
+
#endif /* _HAPROXY_HTTP_H */
/*
diff --git a/src/h3.c b/src/h3.c
index 8f96418..ef84f27 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -133,6 +133,7 @@ DECLARE_STATIC_POOL(pool_head_h3c, "h3c", sizeof(struct h3c));
#define H3_SF_UNI_INIT 0x00000001 /* stream type not parsed for unidirectional stream */
#define H3_SF_UNI_NO_H3 0x00000002 /* unidirectional stream does not carry H3 frames */
+#define H3_SF_HAVE_CLEN 0x00000004 /* content-length header is present */
struct h3s {
struct h3c *h3c;
@@ -142,6 +143,9 @@ struct h3s {
int demux_frame_len;
int demux_frame_type;
+ unsigned long long body_len; /* known request body length from content-length header if present */
+ unsigned long long data_len; /* total length of all parsed DATA */
+
int flags;
};
@@ -325,6 +329,47 @@ static int h3_is_frame_valid(struct h3c *h3c, struct qcs *qcs, uint64_t ftype)
}
}
+/* Check from stream <qcs> that length of all DATA frames does not exceed with
+ * a previously parsed content-length header. <fin> must be set for the last
+ * data of the stream so that length of DATA frames must be equal to the
+ * content-length.
+ *
+ * This must only be called for a stream with H3_SF_HAVE_CLEN flag.
+ *
+ * Return 0 on valid else non-zero.
+ */
+static int h3_check_body_size(struct qcs *qcs, int fin)
+{
+ struct h3s *h3s = qcs->ctx;
+ int ret = 0;
+ TRACE_ENTER(H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+
+ /* Reserved for streams with a previously parsed content-length header. */
+ BUG_ON(!(h3s->flags & H3_SF_HAVE_CLEN));
+
+ /* RFC 9114 4.1.2. Malformed Requests and Responses
+ *
+ * A request or response that is defined as having content when it
+ * contains a Content-Length header field (Section 8.6 of [HTTP]) is
+ * malformed if the value of the Content-Length header field does not
+ * equal the sum of the DATA frame lengths received.
+ *
+ * TODO for backend support
+ * A response that is
+ * defined as never having content, even when a Content-Length is
+ * present, can have a non-zero Content-Length header field even though
+ * no content is included in DATA frames.
+ */
+ if (h3s->data_len > h3s->body_len ||
+ (fin && h3s->data_len < h3s->body_len)) {
+ TRACE_ERROR("Content-length does not match DATA frame size", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
+ ret = -1;
+ }
+
+ TRACE_LEAVE(H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+ return ret;
+}
+
/* Parse from buffer <buf> a H3 HEADERS frame of length <len>. Data are copied
* in a local HTX buffer and transfer to the stream connector layer. <fin> must be
* set if this is the last data to transfer from this stream.
@@ -343,8 +388,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
struct http_hdr list[global.tune.max_http_hdr];
unsigned int flags = HTX_SL_F_NONE;
struct ist meth = IST_NULL, path = IST_NULL;
- //struct ist scheme = IST_NULL, authority = IST_NULL;
- struct ist authority = IST_NULL;
+ struct ist scheme = IST_NULL, authority = IST_NULL;
int hdr_idx, ret;
int cookie = -1, last_cookie = -1, i;
@@ -379,41 +423,113 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
if (ret < 0) {
TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
h3c->err = -ret;
- return -1;
+ len = -1;
+ goto out;
}
qc_get_buf(qcs, &htx_buf);
- BUG_ON(!b_size(&htx_buf));
+ BUG_ON(!b_size(&htx_buf)); /* TODO */
htx = htx_from_buf(&htx_buf);
/* first treat pseudo-header to build the start line */
hdr_idx = 0;
while (1) {
- if (isteq(list[hdr_idx].n, ist("")))
+ /* RFC 9114 4.3. HTTP Control Data
+ *
+ * Endpoints MUST treat a request or response that contains
+ * undefined or invalid pseudo-header fields as malformed.
+ *
+ * All pseudo-header fields MUST appear in the header section before
+ * regular header fields. Any request or response that contains a
+ * pseudo-header field that appears in a header section after a regular
+ * header field MUST be treated as malformed.
+ */
+
+ /* Stop at first non pseudo-header. */
+ if (!istmatch(list[hdr_idx].n, ist(":")))
break;
- if (istmatch(list[hdr_idx].n, ist(":"))) {
- /* pseudo-header */
- if (isteq(list[hdr_idx].n, ist(":method")))
- meth = list[hdr_idx].v;
- else if (isteq(list[hdr_idx].n, ist(":path")))
- path = list[hdr_idx].v;
- //else if (isteq(list[hdr_idx].n, ist(":scheme")))
- // scheme = list[hdr_idx].v;
- else if (isteq(list[hdr_idx].n, ist(":authority")))
- authority = list[hdr_idx].v;
+ /* pseudo-header. Malformed name with uppercase character or
+ * invalid token will be rejected in the else clause.
+ */
+ if (isteq(list[hdr_idx].n, ist(":method"))) {
+ if (isttest(meth)) {
+ TRACE_ERROR("duplicated method pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+
+ if (!istlen(list[hdr_idx].v) || http_method_has_forbidden_char(list[hdr_idx].v)) {
+ TRACE_ERROR("invalid method pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+
+ meth = list[hdr_idx].v;
+ }
+ else if (isteq(list[hdr_idx].n, ist(":path"))) {
+ if (isttest(path)) {
+ TRACE_ERROR("duplicated path pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+ path = list[hdr_idx].v;
+ }
+ else if (isteq(list[hdr_idx].n, ist(":scheme"))) {
+ if (isttest(scheme)) {
+ /* duplicated pseudo-header */
+ TRACE_ERROR("duplicated scheme pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+
+ if (!http_validate_scheme(list[hdr_idx].v)) {
+ TRACE_ERROR("invalid scheme pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+
+ scheme = list[hdr_idx].v;
+ }
+ else if (isteq(list[hdr_idx].n, ist(":authority"))) {
+ if (isttest(authority)) {
+ TRACE_ERROR("duplicated authority pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+ authority = list[hdr_idx].v;
+ }
+ else {
+ TRACE_ERROR("unknown pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
}
++hdr_idx;
}
+ if (!istmatch(meth, ist("CONNECT"))) {
+ /* RFC 9114 4.3.1. Request Pseudo-Header Fields
+ *
+ * All HTTP/3 requests MUST include exactly one value for the :method,
+ * :scheme, and :path pseudo-header fields, unless the request is a
+ * CONNECT request; see Section 4.4.
+ */
+ if (!isttest(meth) || !isttest(scheme) || !isttest(path)) {
+ TRACE_ERROR("missing mandatory pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+ }
+
flags |= HTX_SL_F_VER_11;
flags |= HTX_SL_F_XFER_LEN;
sl = htx_add_stline(htx, HTX_BLK_REQ_SL, flags, meth, path, ist("HTTP/3.0"));
if (!sl) {
h3c->err = H3_INTERNAL_ERROR;
- return -1;
+ len = -1;
+ goto out;
}
if (fin)
@@ -425,16 +541,22 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
htx_add_header(htx, ist("host"), authority);
/* now treat standard headers */
- hdr_idx = 0;
while (1) {
if (isteq(list[hdr_idx].n, ist("")))
break;
+ if (istmatch(list[hdr_idx].n, ist(":"))) {
+ TRACE_ERROR("pseudo-header field after fields", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+
for (i = 0; i < list[hdr_idx].n.len; ++i) {
const char c = list[hdr_idx].n.ptr[i];
if ((uint8_t)(c - 'A') < 'Z' - 'A' || !HTTP_IS_TOKEN(c)) {
TRACE_ERROR("invalid characters in field name", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
- return -1;
+ len = -1;
+ goto out;
}
}
@@ -442,29 +564,54 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
http_cookie_register(list, hdr_idx, &cookie, &last_cookie);
continue;
}
+ else if (isteq(list[hdr_idx].n, ist("content-length"))) {
+ ret = http_parse_cont_len_header(&list[hdr_idx].v,
+ &h3s->body_len,
+ h3s->flags & H3_SF_HAVE_CLEN);
+ if (ret < 0) {
+ TRACE_ERROR("invalid content-length", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+ len = -1;
+ goto out;
+ }
+ else if (!ret) {
+ /* Skip duplicated value. */
+ ++hdr_idx;
+ continue;
+ }
- if (!istmatch(list[hdr_idx].n, ist(":")))
- htx_add_header(htx, list[hdr_idx].n, list[hdr_idx].v);
+ h3s->flags |= H3_SF_HAVE_CLEN;
+ /* This will fail if current frame is the last one and
+ * content-length is not null.
+ */
+ if (h3_check_body_size(qcs, fin)) {
+ len = -1;
+ goto out;
+ }
+ }
+ htx_add_header(htx, list[hdr_idx].n, list[hdr_idx].v);
++hdr_idx;
}
if (cookie >= 0) {
if (http_cookie_merge(htx, list, cookie)) {
h3c->err = H3_INTERNAL_ERROR;
- return -1;
+ len = -1;
+ goto out;
}
}
htx_add_endof(htx, HTX_BLK_EOH);
- htx_to_buf(htx, &htx_buf);
-
if (fin)
htx->flags |= HTX_FL_EOM;
+ htx_to_buf(htx, &htx_buf);
+ htx = NULL;
+
if (!qc_attach_sc(qcs, &htx_buf)) {
h3c->err = H3_INTERNAL_ERROR;
- return -1;
+ len = -1;
+ goto out;
}
/* RFC 9114 5.2. Connection Shutdown
@@ -480,11 +627,18 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
if (qcs->id >= h3c->id_goaway)
h3c->id_goaway = qcs->id + 4;
+ out:
+ /* HTX may be non NULL if error before previous htx_to_buf(). */
+ if (htx)
+ htx_to_buf(htx, &htx_buf);
+
/* buffer is transferred to the stream connector and set to NULL
* except on stream creation error.
*/
- b_free(&htx_buf);
- offer_buffers(NULL, 1);
+ if (b_size(&htx_buf)) {
+ b_free(&htx_buf);
+ offer_buffers(NULL, 1);
+ }
TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
return len;
@@ -683,8 +837,8 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
uint64_t ftype, flen;
char last_stream_frame = 0;
- /* Work on a copy of <rxbuf> */
if (!h3s->demux_frame_len) {
+ /* Switch to a new frame. */
size_t hlen = h3_decode_frm_header(&ftype, &flen, b);
if (!hlen)
break;
@@ -696,6 +850,15 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
h3s->demux_frame_len = flen;
total += hlen;
+ /* Check that content-length is not exceeded on a new DATA frame. */
+ if (ftype == H3_FT_DATA) {
+ h3s->data_len += flen;
+ if (h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin)) {
+ qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
+ return -1;
+ }
+ }
+
if (!h3_is_frame_valid(h3c, qcs, ftype)) {
qcc_emit_cc_app(qcs->qcc, H3_FRAME_UNEXPECTED, 1);
return -1;
@@ -725,6 +888,12 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
break;
}
+ /* Check content-length equality with DATA frames length on the last frame. */
+ if (fin && h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin)) {
+ qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
+ return -1;
+ }
+
last_stream_frame = (fin && flen == b_data(b));
h3_inc_frame_type_cnt(h3c->prx_counters, ftype);
@@ -1110,6 +1279,8 @@ static int h3_attach(struct qcs *qcs, void *conn_ctx)
h3s->demux_frame_len = 0;
h3s->demux_frame_type = 0;
+ h3s->body_len = 0;
+ h3s->data_len = 0;
h3s->flags = 0;
if (quic_stream_is_bidi(qcs->id)) {
--
2.33.0

6
haproxy.spec
View File

@ -5,7 +5,7 @@
Name: haproxy
Version: 2.6.6
Release: 13
Release: 14
Summary: The Reliable, High Performance TCP/HTTP Load Balancer
License: GPLv2+
@ -40,6 +40,7 @@ Patch19: backport-BUG-MINOR-server-source-interface-ignored-from-defau.
Patch20: backport-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
Patch21: fix-timehopping-in-freq_ctr_total.patch
Patch22: backport-BUG-MEDIUM-stream-Prevent-mux-upgrades-if-client-con.patch
Patch23: CVE-2024-53008.patch
BuildRequires: gcc lua-devel pcre2-devel openssl-devel systemd-devel systemd libatomic
%ifarch sw_64
@ -144,6 +145,9 @@ exit 0
%{_mandir}/man1/*
%changelog
* Tue Dec 10 2024 wangkai <13474090681@163.com> - 2.6.6-14
- Fix CVE-2024-53008
* Thu Nov 21 2024 xinghe <xinghe2@h-partners.com> - 2.6.6-13
- Type:bugfix
- CVE:NA