BUG/MAJOR: h3: reject H3 truncated frames

HTTP/3 frames contain a length header. If a frame is truncated, the
connection must be immediately closed with H3_FRAME_ERROR.

This patch adds missing check for truncated frames when FIN is received.
This must be tested in two locations. First, a check is added in the
standalone FIN dedicated block. Second, another check is implemented in
the default parsing while loop, tested on every iteration as large DATA
frames can be read as streaming content.

These checks are crucial to prevent potential request content smuggling.

Thanks to Rajat Raghav for its report which reveal this issue.

This must be backported to all stable releases.

diff --git a/src/h3.c b/src/h3.c
index bc49ef2b43aa66b7eb69d269e316fd951c47bb76..aab7666b027c387a0487373c8c050d1130bcaf19 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -171,8 +171,8 @@ struct h3s {
 
        enum h3s_t type;
        enum h3s_st_req st_req; /* only used for request streams */
-       uint64_t demux_frame_len;
-       uint64_t demux_frame_type;
+       uint64_t demux_frame_len;  /* current parser frame remaining content to read */
+       uint64_t demux_frame_type; /* current parser 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 */
@@ -1915,9 +1915,22 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                goto err;
        }
 
-       if (!b_data(b) && fin && quic_stream_is_bidi(qcs->id)) {
+       if (!b_data(b) && fin) {
                TRACE_PROTO("received FIN without data", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
 
+               /* RFC 9114 7.1. Frame Layout
+                *
+                * When a stream terminates cleanly, if the last frame on the stream was
+                * truncated, this MUST be treated as a connection error of type
+                * H3_FRAME_ERROR.
+                */
+               if (h3s->demux_frame_len) {
+                       TRACE_ERROR("truncated frame on FIN", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                       qcc_set_error(qcs->qcc, H3_ERR_FRAME_ERROR, 1, muxc_tevt_type_proto_err);
+                       qcc_report_glitch(qcs->qcc, 1);
+                       goto err;
+               }
+
                /* FIN received, ensure body length is conform to any content-length header. */
                if ((h3s->flags & H3_SF_HAVE_CLEN) && h3_check_body_size(qcs, 1)) {
                        qcc_abort_stream_read(qcs, h3s->err);
@@ -1925,7 +1938,7 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        goto done;
                }
 
-               if (qcs_http_handle_standalone_fin(qcs)) {
+               if (quic_stream_is_bidi(qcs->id) && qcs_http_handle_standalone_fin(qcs)) {
                        TRACE_ERROR("cannot set EOM", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
                        qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1,
                                      muxc_tevt_type_internal_err);
@@ -1979,6 +1992,19 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                break;
                }
 
+               /* RFC 9114 7.1. Frame Layout
+                *
+                * When a stream terminates cleanly, if the last frame on the stream was
+                * truncated, this MUST be treated as a connection error of type
+                * H3_FRAME_ERROR.
+                */
+               if (fin && h3s->demux_frame_len > b_data(b)) {
+                       TRACE_ERROR("truncated frame on FIN", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                       qcc_set_error(qcs->qcc, H3_ERR_FRAME_ERROR, 1, muxc_tevt_type_proto_err);
+                       qcc_report_glitch(qcs->qcc, 1);
+                       goto err;
+               }
+
                flen = h3s->demux_frame_len;
                ftype = h3s->demux_frame_type;