From d2c5ee665ea2d812056cb24edbca31dc42e5d246 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle 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 --- src/h3.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/src/h3.c b/src/h3.c index 3644230524..d24b3de5f4 100644 --- a/src/h3.c +++ b/src/h3.c @@ -139,6 +139,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; @@ -148,6 +149,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; }; @@ -331,6 +335,47 @@ static int h3_is_frame_valid(struct h3c *h3c, struct qcs *qcs, uint64_t ftype) } } +/* Check from stream that length of all DATA frames does not exceed with + * a previously parsed content-length header. 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 a H3 HEADERS frame of length . Data are copied * in a local HTX buffer and transfer to the stream connector layer. must be * set if this is the last data to transfer from this stream. @@ -501,6 +546,27 @@ 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); + return -1; + } + else if (!ret) { + /* Skip duplicated value. */ + ++hdr_idx; + continue; + } + + 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)) + return -1; + } htx_add_header(htx, list[hdr_idx].n, list[hdr_idx].v); ++hdr_idx; @@ -740,8 +806,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 */ if (!h3s->demux_frame_len) { + /* Switch to a new frame. */ size_t hlen = h3_decode_frm_header(&ftype, &flen, b); if (!hlen) break; @@ -753,6 +819,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; @@ -782,6 +857,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); @@ -1167,6 +1248,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)) {