Skip to content

Commit 4fd5caf

Browse files
committed
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.
1 parent d26bd9f commit 4fd5caf

File tree

1 file changed

+13
-4
lines changed

1 file changed

+13
-4
lines changed

src/htx.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,9 +733,11 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
733733
{
734734
struct htx_blk *blk, *last_dstblk;
735735
size_t ret = 0;
736+
uint32_t max, last_dstblk_sz;
736737
int dst_full = 0;
737738

738739
last_dstblk = NULL;
740+
last_dstblk_sz = 0;
739741
for (blk = htx_get_head_blk(src); blk && count; blk = htx_get_next_blk(src, blk)) {
740742
struct ist v;
741743
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
751753
type != HTX_BLK_HDR && type != HTX_BLK_EOH)
752754
break;
753755

756+
max = htx_get_max_blksz(dst, count);
757+
if (!max)
758+
break;
759+
754760
sz = htx_get_blksz(blk);
755761
switch (type) {
756762
case HTX_BLK_DATA:
757763
v = htx_get_blk_value(src, blk);
758-
if (v.len > count)
759-
v.len = count;
764+
if (v.len > max)
765+
v.len = max;
760766
v.len = htx_add_data(dst, v);
761767
if (!v.len) {
762768
dst_full = 1;
763769
goto stop;
764770
}
765771
last_dstblk = htx_get_tail_blk(dst);
772+
last_dstblk_sz = v.len;
766773
count -= sizeof(*blk) + v.len;
767774
ret += sizeof(*blk) + v.len;
768775
if (v.len != sz) {
@@ -772,7 +779,7 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
772779
break;
773780

774781
default:
775-
if (sz > count) {
782+
if (sz > max) {
776783
dst_full = 1;
777784
goto stop;
778785
}
@@ -784,12 +791,14 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla
784791
}
785792
last_dstblk->info = blk->info;
786793
htx_memcpy(htx_get_blk_ptr(dst, last_dstblk), htx_get_blk_ptr(src, blk), sz);
794+
last_dstblk_sz = sz;
787795
count -= sizeof(*blk) + sz;
788796
ret += sizeof(*blk) + sz;
789797
break;
790798
}
791799

792800
last_dstblk = NULL; /* Reset last_dstblk because it was fully copied */
801+
last_dstblk_sz = 0;
793802
}
794803
stop:
795804
/* 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
848857
* cut the source block accordingly
849858
*/
850859
if (last_dstblk && blk2 && htx_get_blk_type(blk2) == HTX_BLK_DATA) {
851-
htx_cut_data_blk(src, blk2, htx_get_blksz(last_dstblk));
860+
htx_cut_data_blk(src, blk2, last_dstblk_sz);
852861
}
853862
}
854863
}

0 commit comments

Comments
 (0)