BUG/MEDIUM: h3: reject unaligned frames except DATA

HTTP/3 parser cannot deal with unaligned frames, except for DATA. As it
was expected that such case would not occur, a simple BUG_ON() was
written to protect HEADERS parsing.

First, this BUG_ON() was incorrectly written due an incorrect operator
'>=' vs '>' when checking if data wraps. Thus this patch correct it.

However this correction is not sufficient as it still possible to handle
a large unaligned HEADERS frame, which would trigger this BUG_ON(). This
is very unlikely as HEADERS is the first received frame on a request
stream, but not completely impossible. As HTTP/3 frame header (type +
length) is parsed first and removed, this leaves a small gap at the
buffer beginning. If this small gap is then filled with the remaining
frame payload, it would result in unaligned data. Also, trailers are
also sensitive here as in this case a HEADERS frame is handled after
other frames.

The objective of this patch is to ensure that an unaligned frame is now
handled in a safe way. This is extend to all HTTP/3 frames (except DATA)
and not only to HEADERS type. Parsing is interrupted if frame payload is
wrapping in the buffer. This should never happen except maybe with some
weird clients, so the connection is closed with H3_EXCESSIVE_LOAD error.

This approach is considered the safest one, in particular for backport
purpose. In the future, realign operation via copy may be implemented
instead if considered as useful.

This must be backported up to 2.6.

diff --git a/src/h3.c b/src/h3.c
index e10dea69cabdc26e961cd17169bc132aac06d306..4e5d52f5b46ffef137dfcacbf30103ec2977fd68 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -641,7 +641,7 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        /* TODO support trailer parsing in this function */
 
        /* TODO support buffer wrapping */
-       BUG_ON(b_head(buf) + len >= b_wrap(buf));
+       BUG_ON(b_head(buf) + len > b_wrap(buf));
        ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
                            list, sizeof(list) / sizeof(list[0]));
        if (ret < 0) {
@@ -1142,7 +1142,7 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
 
        /* TODO support buffer wrapping */
-       BUG_ON(b_head(buf) + len >= b_wrap(buf));
+       BUG_ON(b_head(buf) + len > b_wrap(buf));
        ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
                            list, sizeof(list) / sizeof(list[0]));
        if (ret < 0) {
@@ -1391,7 +1391,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
        TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
 
        /* TODO support buffer wrapping */
-       BUG_ON(b_head(buf) + len >= b_wrap(buf));
+       BUG_ON(b_head(buf) + len > b_wrap(buf));
        ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
                            list, sizeof(list) / sizeof(list[0]));
        if (ret < 0) {
@@ -1818,10 +1818,11 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                flen = h3s->demux_frame_len;
                ftype = h3s->demux_frame_type;
 
-               /* Do not demux incomplete frames except H3 DATA which can be
-                * fragmented in multiple HTX blocks.
+               /* Current HTTP/3 parser can currently only parse fully
+                * received and aligned frames. The only exception is for DATA
+                * frames as they can frequently be larger than bufsize.
                 */
-               if (flen > b_data(b) && ftype != H3_FT_DATA) {
+               if (ftype != H3_FT_DATA) {
                        /* Reject frames bigger than bufsize.
                         *
                         * TODO HEADERS should in complement be limited with H3
@@ -1834,7 +1835,20 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                qcc_report_glitch(qcs->qcc, 1);
                                goto err;
                        }
-                       break;
+
+                       /* TODO extend parser to support the realignment of a frame. */
+                       if (b_head(b) + b_data(b) > b_wrap(b)) {
+                               TRACE_ERROR("cannot parse unaligned data frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                               qcc_set_error(qcs->qcc, H3_ERR_EXCESSIVE_LOAD, 1);
+                               qcc_report_glitch(qcs->qcc, 1);
+                               goto err;
+                       }
+
+                       /* Only parse full HTTP/3 frames. */
+                       if (flen > b_data(b)) {
+                               TRACE_PROTO("pause parsing on incomplete payload", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                               break;
+                       }
                }
 
                last_stream_frame = (fin && flen == b_data(b));