]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: h3: reject header values containing invalid chars
authorWilly Tarreau <w@1wt.eu>
Tue, 8 Aug 2023 15:18:27 +0000 (17:18 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 8 Aug 2023 17:02:24 +0000 (19:02 +0200)
In practice it's exactly the same for h3 as 54f53ef7c ("BUG/MAJOR: h2:
reject header values containing invalid chars") was for h2: we must
make sure never to accept NUL/CR/LF in any header value because this
may be used to construct splitted headers on the backend. Hence we
apply the same solution. Here pseudo-headers, headers and trailers are
checked separately, which explains why we have 3 locations instead of
2 for h2 (+1 for response which we don't have here).

This is marked major for consistency and due to the impact if abused,
but the reality is that at the time of writing, this problem is limited
by the scarcity of the tools which would permit to build such a request
in the first place. But this may change over time.

This must be backported to 2.6. This depends on the following commit
that exposes the filtering function:

    REORG: http: move has_forbidden_char() from h2.c to http.h

src/h3.c

index b10851baa1b445cef5c1a50de3a210314f099935..95a8303afebe722cabaf7114baf0c8b78b862868 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -418,6 +418,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        struct ist scheme = IST_NULL, authority = IST_NULL;
        int hdr_idx, ret;
        int cookie = -1, last_cookie = -1, i;
+       const char *ctl;
 
        /* RFC 9114 4.1.2. Malformed Requests and Responses
         *
@@ -481,6 +482,25 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
                if (!istmatch(list[hdr_idx].n, ist(":")))
                        break;
 
+               /* RFC 9114 10.3 Intermediary-Encapsulation Attacks
+                *
+                * While most values that can be encoded will not alter field
+                * parsing, carriage return (ASCII 0x0d), line feed (ASCII 0x0a),
+                * and the null character (ASCII 0x00) might be exploited by an
+                * attacker if they are translated verbatim. Any request or
+                * response that contains a character not permitted in a field
+                * value MUST be treated as malformed
+                */
+
+               /* look for forbidden control characters in the pseudo-header value */
+               ctl = ist_find_ctl(list[hdr_idx].v);
+               if (unlikely(ctl) && http_header_has_forbidden_char(list[hdr_idx].v, ctl)) {
+                       TRACE_ERROR("control character present in pseudo-header value", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+                       h3s->err = H3_MESSAGE_ERROR;
+                       len = -1;
+                       goto out;
+               }
+
                /* pseudo-header. Malformed name with uppercase character or
                 * invalid token will be rejected in the else clause.
                 */
@@ -591,6 +611,26 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
                        }
                }
 
+
+               /* RFC 9114 10.3 Intermediary-Encapsulation Attacks
+                *
+                * While most values that can be encoded will not alter field
+                * parsing, carriage return (ASCII 0x0d), line feed (ASCII 0x0a),
+                * and the null character (ASCII 0x00) might be exploited by an
+                * attacker if they are translated verbatim. Any request or
+                * response that contains a character not permitted in a field
+                * value MUST be treated as malformed
+                */
+
+               /* look for forbidden control characters in the header value */
+               ctl = ist_find_ctl(list[hdr_idx].v);
+               if (unlikely(ctl) && http_header_has_forbidden_char(list[hdr_idx].v, ctl)) {
+                       TRACE_ERROR("control character present in header value", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+                       h3s->err = H3_MESSAGE_ERROR;
+                       len = -1;
+                       goto out;
+               }
+
                if (isteq(list[hdr_idx].n, ist("cookie"))) {
                        http_cookie_register(list, hdr_idx, &cookie, &last_cookie);
                        ++hdr_idx;
@@ -738,6 +778,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
        struct htx_sl *sl;
        struct http_hdr list[global.tune.max_http_hdr];
        int hdr_idx, ret;
+       const char *ctl;
        int i;
 
        TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
@@ -818,6 +859,25 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
                        goto out;
                }
 
+               /* RFC 9114 10.3 Intermediary-Encapsulation Attacks
+                *
+                * While most values that can be encoded will not alter field
+                * parsing, carriage return (ASCII 0x0d), line feed (ASCII 0x0a),
+                * and the null character (ASCII 0x00) might be exploited by an
+                * attacker if they are translated verbatim. Any request or
+                * response that contains a character not permitted in a field
+                * value MUST be treated as malformed
+                */
+
+               /* look for forbidden control characters in the trailer value */
+               ctl = ist_find_ctl(list[hdr_idx].v);
+               if (unlikely(ctl) && http_header_has_forbidden_char(list[hdr_idx].v, ctl)) {
+                       TRACE_ERROR("control character present in trailer value", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+                       h3s->err = H3_MESSAGE_ERROR;
+                       len = -1;
+                       goto out;
+               }
+
                if (!htx_add_trailer(htx, list[hdr_idx].n, list[hdr_idx].v)) {
                        TRACE_ERROR("cannot add trailer", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
                        h3c->err = H3_INTERNAL_ERROR;