BUG/MINOR: h3: reject truncated frames with mandatory payload

Most of HTTP/3 control frames contain a fix set of fields. If a frame is
truncated, an implementation must close the connection with
H3_FRAME_ERROR code.

This patch adds some basic checks on CANCEL_PUSH, PUSH_PROMISE and
MAX_PUSH_ID : if the length is null, H3_FRAME_ERROR code is used.

This bug can be considered minor as push is not supported by haproxy.
They were completely ignored before backend support as only a server is
expected to emit them.

This must be backported up to 3.3.

diff --git a/src/h3.c b/src/h3.c
index 50f90a3d173b1d6accfec5e885d88f1b94d6d827..dbf9dbc76e360150f362469941f68a4dace28de0 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1691,7 +1691,8 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
        return -1;
 }
 
-/* Parse a SETTINGS frame of length <len> of payload <buf>.
+/* Parse a SETTINGS frame of length <len> and of payload <buf>. An empty frame
+ * is supported : this will leave the parameters to their default value.
  *
  * Returns the number of consumed bytes or a negative error code.
  */
@@ -1783,6 +1784,15 @@ static ssize_t h3_parse_goaway_frm(struct h3c *h3c, const struct buffer *buf,
 
        b = b_make(b_orig(buf), b_size(buf), b_head_ofs(buf), len);
        if (!b_quic_dec_int(&id, &b, &ret)) {
+               /* RFC 9114 7.1. Frame Layout
+                *
+                * Each frame's payload MUST contain exactly the fields identified in
+                * its description. A frame payload that contains additional bytes after
+                * the identified fields or a frame payload that terminates before the
+                * end of the identified fields MUST be treated as a connection error of
+                * type H3_FRAME_ERROR.
+                */
+               TRACE_ERROR("truncated GOAWAY frame", H3_EV_RX_FRAME, h3c->qcc->conn);
                h3c->err = H3_ERR_FRAME_ERROR;
                qcc_report_glitch(h3c->qcc, 1);
                return -1;
@@ -2043,6 +2053,21 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        }
                        break;
                case H3_FT_CANCEL_PUSH:
+                       if (unlikely(!flen)) {
+                               /* RFC 9114 7.1. Frame Layout
+                                *
+                                * Each frame's payload MUST contain exactly the fields identified in
+                                * its description. A frame payload that contains additional bytes after
+                                * the identified fields or a frame payload that terminates before the
+                                * end of the identified fields MUST be treated as a connection error of
+                                * type H3_FRAME_ERROR.
+                                */
+                               TRACE_ERROR("truncated CANCEL_PUSH frame", 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;
+                       }
+
                        if (!conn_is_back(qcs->qcc->conn)) {
                                /* RFC 9114 7.2.3. CANCEL_PUSH
                                 *
@@ -2082,6 +2107,21 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        /* h3_check_frame_valid() must reject on server side. */
                        BUG_ON(!conn_is_back(qcs->qcc->conn));
 
+                       if (unlikely(!flen)) {
+                               /* RFC 9114 7.1. Frame Layout
+                                *
+                                * Each frame's payload MUST contain exactly the fields identified in
+                                * its description. A frame payload that contains additional bytes after
+                                * the identified fields or a frame payload that terminates before the
+                                * end of the identified fields MUST be treated as a connection error of
+                                * type H3_FRAME_ERROR.
+                                */
+                               TRACE_ERROR("truncated PUSH_PROMISE frame", 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;
+                       }
+
                        /* RFC 9114 7.2.5. PUSH_PROMISE
                         *
                         * A client MUST treat
@@ -2095,6 +2135,21 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        /* h3_check_frame_valid() must reject on client side. */
                        BUG_ON(conn_is_back(qcs->qcc->conn));
 
+                       if (unlikely(!flen)) {
+                               /* RFC 9114 7.1. Frame Layout
+                                *
+                                * Each frame's payload MUST contain exactly the fields identified in
+                                * its description. A frame payload that contains additional bytes after
+                                * the identified fields or a frame payload that terminates before the
+                                * end of the identified fields MUST be treated as a connection error of
+                                * type H3_FRAME_ERROR.
+                                */
+                               TRACE_ERROR("truncated MAX_PUSH_ID frame", 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;
+                       }
+
                        /* Not supported. */
                        ret = flen;
                        break;