BUG/MINOR: h3: properly reject too long header responses

When encoding HTX to HTTP/3 headers on the response path, a bunch of
ABORT_NOW() where used when buffer room was not enough. In most cases
this is safe as output buffer has just been allocated and so is empty at
the start of the function. However, with a header list longer than a
whole buffer, this would cause an unexpected crash.

Fix this by removing ABORT_NOW() statement with proper error return
path. For the moment, this would cause the whole connection to be close
rather than the stream only. This may be further improved in the future.

Also remove ABORT_NOW() when encoding frame length at the end of headers
or trailers encoding. Buffer room is sufficient as it was already
checked prior in the same function.

This should be backported up to 2.6. Special care should be handled
however as this code path has changed frequently :
* for 2.9 and older, the extra following statement must be inserted
  prior each newly added goto statement :
  h3c->err = H3_INTERNAL_ERROR;
* for 2.6, trailers support is not implemented. As such, related chunks
  should just be ignored when backporting.

diff --git a/src/h3.c b/src/h3.c
index 68e302dfaf0f399919dadf781a03e9fe934801f2..2898b9e41ed273deb0ea2b98465a1268075bd1ac 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1596,6 +1596,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                        hdr++;
                }
                else {
+                       /* Unhandled HTX block type. */
                        ABORT_NOW();
                        goto err;
                }
@@ -1615,9 +1616,8 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                goto end;
        }
 
-       /* At least 5 bytes to store frame type + length as a varint max size */
-       if (b_room(res) < 5)
-               ABORT_NOW();
+       /* Buffer allocated just now : must be enough for frame type + length as a max varint size */
+       BUG_ON(b_room(res) < 5);
 
        b_reset(&outbuf);
        outbuf = b_make(b_tail(res), b_contig_space(res), 0, 0);
@@ -1627,9 +1627,10 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
        TRACE_DATA("encoding HEADERS frame", H3_EV_TX_FRAME|H3_EV_TX_HDR,
                   qcs->qcc->conn, qcs);
        if (qpack_encode_field_section_line(&headers_buf))
-               ABORT_NOW();
+               goto err;
        if (qpack_encode_int_status(&headers_buf, status)) {
-               TRACE_ERROR("invalid status code", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
+               /* TODO handle invalid status code VS no buf space left */
+               TRACE_ERROR("error during status code encoding", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
                goto err;
        }
 
@@ -1661,7 +1662,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                }
 
                if (qpack_encode_header(&headers_buf, list[hdr].n, list[hdr].v))
-                       ABORT_NOW();
+                       goto err;
        }
 
        /* Now that all headers are encoded, we are certain that res buffer is
@@ -1670,8 +1671,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
        frame_length_size = quic_int_getsize(b_data(&headers_buf));
        res->head += 4 - frame_length_size;
        b_putchr(res, 0x01); /* h3 HEADERS frame type */
-       if (!b_quic_enc_int(res, b_data(&headers_buf), 0))
-               ABORT_NOW();
+       b_quic_enc_int(res, b_data(&headers_buf), 0);
        b_add(res, b_data(&headers_buf));
 
        ret = 0;
@@ -1839,8 +1839,7 @@ static int h3_resp_trailers_send(struct qcs *qcs, struct htx *htx)
                TRACE_DATA("encoding TRAILERS frame", H3_EV_TX_FRAME|H3_EV_TX_HDR,
                           qcs->qcc->conn, qcs);
                b_putchr(res, 0x01); /* h3 HEADERS frame type */
-               if (!b_quic_enc_int(res, b_data(&headers_buf), 8))
-                       ABORT_NOW();
+               b_quic_enc_int(res, b_data(&headers_buf), 8);
                b_add(res, b_data(&headers_buf));
        }