On Wed, Apr 08, 2026 at 08:58:10PM +0200, Willy Tarreau wrote:
> Hi Greg,
>
> Quick update on this one.
>
> On Tue, Apr 07, 2026 at 09:48:14AM +0200, Greg Kroah-Hartman wrote:
> > slz_encode() has no output buffer bound parameter; it writes
> > unconditionally to strm->outbuf. The function comment claims "output
> > result may be up to 5 bytes larger than the input", but this only
> > holds when the bit9>=52 heuristic at slz.c:634 forces a switch to
> > stored blocks. An attacker can defeat the heuristic by interleaving
> > short (4-byte) matches between runs of <= 51 bytes >= 144, which are
> > encoded with 9-bit fixed Huffman codes.
> >
> > With this pattern, ~51 9-bit literals + one ~16-bit match per 55
> > input bytes = ~8.6 bits/byte. Pure literals >= 144 with no matches
> > expand by 12.5%. The comp_http_payload input cap was b_size(&trash)
> > with no headroom, so a 16336-byte HTX DATA block of crafted data
> > expands to ~17600+ bytes into a 16384-byte trash buffer.
> >
> > The trash buffer is a thread-local heap allocation (chunk.c:189
> > my_realloc2). The overflow corrupts adjacent heap objects with
> > deflate-encoded bytes whose structure the attacker controls.
> (...)
>
> So the real culprit was (as expected) the SLZ compressor, which I could
> finally fix to honnor the promise of not more than +5 bytes total. This
> was updated in haproxy as commit 3020fde52 ("BUG/MAJOR: slz: always make
> sure to limit fixed output to less than worst case literals"):
>
> https://github.com/haproxy/haproxy/commit/3020fde5258
>
> A few notes, though:
> - the real size increase is in fact 1/16, not 1/8, which is great
> because with the default settings (bufsize 16384, maxrewrite 1024),
> we compress at most 15360 at once into at most 16320, so it fits!
> This explains why I could only cause ASAN bugs (and without ASAN
> it triggers no less than 3 BUG_ON()) with larger bufsize or smaller
> maxrewrite.
>
> - the +5 per 65535 guarantee is already taken into account in haproxy
> when starting to compress (this was what scared me the most), so
> nothing needs to be changed there.
>
> - since the fix is totally different (and at a different place), I
> mentioned you in Reported-by.
>
> Many thanks for this one, it made me lose even more hair by scratching my
> head quite a bit redrawing the compressor's state machine :-) In the end
> for a few chars changed on a single line, no less than 45 lines of
> comments were added, hoping it will help me next time!
Very cool, thanks!
But did you miss the check for htx_replace_blk_value() that this patch
had in comp_http_payload(), pasted here in a whitespace damaged way:
@@ -330,6 +338,8 @@ comp_http_payload(struct stream *s, struct filter *filter,
struct http_msg *msg,
next = htx_remove_blk(htx, blk);
else {
blk = htx_replace_blk_value(htx, blk,
v, ist2(b_head(&trash), b_data(&trash)));
+ if (!blk)
+ goto error;
next = htx_get_next_blk(htx, blk);
}
Isn't that still needed, or can that never fail here?
thanks,
greg k-h
