Hi Cyril, On Tue, Dec 18, 2018 at 09:11:26AM +0100, Cyril Bonté wrote: > There's a bug with cookies and HTX. > When haproxy sends the headers to the backend server, it leaks 6 more bytes > at the end of the cookie value, which is the length of the name "cookie". > > This is due to this part of code : > http://git.haproxy.org/?p=haproxy.git;a=blob;f=src/h2.c;h=1b784fd4aba2dbad91db156612243e93aa34e5f3;hb=HEAD#l533 > > http://git.haproxy.org/?p=haproxy.git;a=blob;f=src/h2.c;h=1b784fd4aba2dbad91db156612243e93aa34e5f3;hb=HEAD#l557 > htx_set_blk_value_len(blk, bs + 2 + vl); > > Here, we set the new block value length, but bs is not the value length, it > the whole block size : > http://git.haproxy.org/?p=haproxy.git;a=blob;f=src/h2.c;h=1b784fd4aba2dbad91db156612243e93aa34e5f3;hb=HEAD#l542 > 542 blk = htx_add_header(htx, ist("cookie"), list[ck].v); > 543 if (!blk) > 544 goto fail; > 545 > 546 fs = htx_free_data_space(htx); > 547 bs = htx_get_blksz(blk); > > I'm not sure how to fix this properly. I'd say we can store list[ck].v in a > "bvl" variable, update this variable while looping on the cookie values, > then set the new block value length according to this result.
Ah you're totally right, good catch! > Or is there a function to explicitely update the block size (kind of > htx_set_blksz) ? I don't know :-) But I find it safer to only adjust the value's length anyway. I agree with your suggestion to keep the total value length, I'll do this. > I prefer to report the bug quickly so you can fix it today. Or I can work on > a patch, but this will not be before the end of the day ;-) No that's perfect, I'll do it right now. Thank you! Willy
