]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: h3: parse content-length and reject invalid messages
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 8 Dec 2022 15:54:42 +0000 (16:54 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 14 Dec 2022 10:37:12 +0000 (11:37 +0100)
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

index 3644230524c5b083d921e856454ae306229d74da..d24b3de5f4f6c57f4c47d8becb25b6e32bfd0fdf 100644 (file)
--- 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 <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.
@@ -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 <rxbuf> */
                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)) {