BUG/MINOR: h3/qpack: deal with too many headers

ensures that we never insert too many entries in a headers input list.
On the decoding side, a new error QPACK_ERR_TOO_LARGE is reported in
this case.

This prevents crash if headers number on a H3 request or response is
superior to tune.http.maxhdr config value. Previously, a crash would
occur in QPACK decoding function.

Note that the process still crashes later with ABORT_NOW() because error
reporting on frame parsing is not implemented for now. It should be
treated with a RESET_STREAM frame in most cases.

This can be backported up to 2.6.

diff --git a/include/haproxy/qpack-dec.h b/include/haproxy/qpack-dec.h
index a5d9ba197d747ead760f0a0e860bf96af89d949f..9239e218360a7c9f6204e529374f19c2175658f7 100644 (file)
--- a/include/haproxy/qpack-dec.h
+++ b/include/haproxy/qpack-dec.h
@@ -33,6 +33,7 @@ enum {
        QPACK_ERR_DB,        /* cannot decode Delta Base prefix field */
        QPACK_ERR_TRUNCATED, /* truncated stream */
        QPACK_ERR_HUFFMAN,   /* huffman decoding error */
+       QPACK_ERR_TOO_LARGE, /* decoded request/response is too large */
 };
 
 struct qpack_dec {
@@ -43,7 +44,7 @@ struct qpack_dec {
 };
 
 int qpack_decode_fs(const unsigned char *buf, uint64_t len, struct buffer *tmp,
-                    struct http_hdr *list);
+                    struct http_hdr *list, int list_size);
 int qpack_decode_enc(struct buffer *buf, void *ctx);
 int qpack_decode_dec(struct buffer *buf, void *ctx);
 

diff --git a/src/h3.c b/src/h3.c
index a18e7d7e2677a678ec743825f02264f8b092095a..de5a114a9941d7271b781cd3b1f90801caa305a0 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -341,8 +341,10 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
 
        /* TODO support buffer wrapping */
        BUG_ON(b_head(buf) + len >= b_wrap(buf));
-       if (qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp, list) < 0)
+       if (qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp,
+                           list, sizeof(list) / sizeof(list[0])) < 0) {
                return -1;
+       }
 
        qc_get_buf(qcs, &htx_buf);
        BUG_ON(!b_size(&htx_buf));
@@ -795,6 +797,8 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                        status = sl->info.res.status;
                }
                else if (type == HTX_BLK_HDR) {
+                       if (unlikely(hdr >= sizeof(list) / sizeof(list[0]) - 1))
+                               goto fail;
                        list[hdr].n = htx_get_blk_name(htx, blk);
                        list[hdr].v = htx_get_blk_value(htx, blk);
                        hdr++;

diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index aac35a64785e39abda0d40a988a3c425adb0a8ae..4ee466d889841f1d0dfb0f8eb4ee4993767a175a 100644 (file)
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -177,15 +177,15 @@ static int qpack_decode_fs_pfx(uint64_t *enc_ric, uint64_t *db, int *sign_bit,
 }
 
 /* Decode a field section from the <raw> buffer of <len> bytes. Each parsed
- * header is inserted into <list> and uses <tmp> as a storage for some elements
- * pointing into it. An end marker is inserted at the end of the list with
- * empty strings as name/value.
+ * header is inserted into <list> of <list_size> entries max and uses <tmp> as
+ * a storage for some elements pointing into it. An end marker is inserted at
+ * the end of the list with empty strings as name/value.
  *
  * Returns 0 on success. In case of error, a negative code QPACK_ERR_* is
  * returned.
  */
 int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
-                    struct http_hdr *list)
+                    struct http_hdr *list, int list_size)
 {
        uint64_t enc_ric, db;
        int s;
@@ -207,6 +207,12 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
                           (unsigned long long)enc_ric, (unsigned long long)db, !!s);
        /* Decode field lines */
        while (len) {
+               if (hdr_idx >= list_size) {
+                       qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
+                       ret = -QPACK_ERR_TOO_LARGE;
+                       goto out;
+               }
+
                /* parse field line representation */
                efl_type = *raw & QPACK_EFL_BITMASK;
                qpack_debug_printf(stderr, "efl_type=0x%02x\n", efl_type);
@@ -464,6 +470,12 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
                qpack_debug_printf(stderr, "\n");
        }
 
+       if (hdr_idx >= list_size) {
+               qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
+               ret = -QPACK_ERR_TOO_LARGE;
+               goto out;
+       }
+
        /* put an end marker */
        list[hdr_idx].n = list[hdr_idx].v = IST_NULL;