BUG/MEDIUM: htx: Fix htx_xfer() to consume more data than expected

When an htx DATA block is partially transfer, we must take care to remove
exactly the copied size. To do so, we must save the size of the last block
value copied and not rely on the last data block after the copy. Indeed,
data can be merged with an existing DATA block, so the last block size can
be larger than the last part copied.

Because of this issue, it is possible to remove more data than
expected. Worse, this could lead to a crash by performing an integer
overflow on the block size.

No backport needed.

diff --git a/src/htx.c b/src/htx.c
index 13b6a0cbc0cb3ea9998389db53f806536648d7d9..8c0ea966a16e94fafc7cf6ce5d28846bc597b204 100644 (file)
--- a/src/htx.c
+++ b/src/htx.c
@@ -733,9 +733,11 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
 {
        struct htx_blk *blk, *last_dstblk;
        size_t ret = 0;
+       uint32_t max, last_dstblk_sz;
        int dst_full = 0;
 
        last_dstblk = NULL;
+       last_dstblk_sz = 0;
        for (blk = htx_get_head_blk(src); blk && count; blk = htx_get_next_blk(src, blk)) {
                struct ist v;
                enum htx_blk_type type;
@@ -751,18 +753,23 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
                    type != HTX_BLK_HDR && type != HTX_BLK_EOH)
                        break;
 
+               max = htx_get_max_blksz(dst, count);
+               if (!max)
+                       break;
+
                sz = htx_get_blksz(blk);
                switch (type) {
                case HTX_BLK_DATA:
                        v = htx_get_blk_value(src, blk);
-                       if (v.len > count)
-                               v.len = count;
+                       if (v.len > max)
+                               v.len = max;
                        v.len = htx_add_data(dst, v);
                        if (!v.len) {
                                dst_full = 1;
                                goto stop;
                        }
                        last_dstblk = htx_get_tail_blk(dst);
+                       last_dstblk_sz = v.len;
                        count -= sizeof(*blk) + v.len;
                        ret += sizeof(*blk) + v.len;
                        if (v.len != sz) {
@@ -772,7 +779,7 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
                        break;
 
                default:
-                       if (sz > count) {
+                       if (sz > max) {
                                dst_full = 1;
                                goto stop;
                        }
@@ -784,12 +791,14 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
                        }
                        last_dstblk->info = blk->info;
                        htx_memcpy(htx_get_blk_ptr(dst, last_dstblk), htx_get_blk_ptr(src, blk), sz);
+                       last_dstblk_sz = sz;
                        count -= sizeof(*blk) + sz;
                        ret += sizeof(*blk) + sz;
                        break;
                }
 
                last_dstblk = NULL; /* Reset last_dstblk because it was fully copied */
+               last_dstblk_sz = 0;
        }
   stop:
        /* Here, if not NULL, <blk> point on the first not fully copied block in
@@ -848,7 +857,7 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
                         * cut the source block accordingly
                         */
                        if (last_dstblk && blk2 && htx_get_blk_type(blk2) == HTX_BLK_DATA) {
-                               htx_cut_data_blk(src, blk2, htx_get_blksz(last_dstblk));
+                               htx_cut_data_blk(src, blk2, last_dstblk_sz);
                        }
                }
        }