From: Willy Tarreau Date: Sun, 26 Jul 2026 21:48:04 +0000 (+0200) Subject: BUG/MINOR: slz: do not append a block to an already finished stream X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=421b3078ed374dc7e285fa46758889c3319831e1;p=thirdparty%2Fhaproxy.git BUG/MINOR: slz: do not append a block to an already finished stream slz_rfc1951_flush() terminates the pending block then emits an empty stored block to byte-align the output. But encode() called with cleared may leave the stream in SLZ_ST_LAST, that is, inside a block whose BFINAL bit has already been sent. Flushing there terminated that block, which completes the deflate stream, and then emitted one more block with BFINAL set again. Those 5 bytes sit past the end of the stream, so the gzip or zlib trailer that finish() appends right after is no longer where the peer expects it: it reads the empty block as the checksum. The data itself decodes correctly, only the check fails, which makes it particularly difficult to diagnose. Raw deflate is not affected as it has no trailer to shift, and a stream that ends in EOB state is not affected either since its queue is empty and flush() returns early. Fuzzing random sequences of encode/flush/finish showed ~2% corrupt streams for gzip and zlib. When the terminated block was the final one there is nothing left to align to, and nothing may be appended, so let's just flush the pending bits and return. This also makes the flush 4 bytes shorter in that case, and the BFINAL bit of the empty block is now always zero, which is what the state guarantees at that point. This must be backported to 1.2. This is libslz upstream commit 12e726390c96a21bd910d02a2fc594bcd743c131. Note: no practical impact in haproxy since we never send streams back- to-back, but some deflate stream decoders might occasionally spot an error. --- diff --git a/src/slz.c b/src/slz.c index 1c406c662..f50946ff1 100644 --- a/src/slz.c +++ b/src/slz.c @@ -853,9 +853,12 @@ int slz_rfc1951_init(struct slz_stream *strm, int level) * queue (up to 31 bits when using the 64-bit queue), plus a possible EOB (7 * bits), plus (BFINAL,BTYPE) (3 bits), which once rounded up to the next byte * make 6 bytes, plus 4 bytes for LEN+NLEN, or a total of 10 bytes in the worst - * case. The number of bytes emitted is returned. It is guaranteed that the - * queue is empty on return. This may cause some overhead by adding needless - * 5-byte blocks if called to often. + * case. If the stream was already completed by a previous call to encode() + * with cleared, the last block is simply terminated and the output + * aligned, since nothing may be appended to a finished stream. The number of + * bytes emitted is returned. It is guaranteed that the queue is empty on + * return. This may cause some overhead by adding needless 5-byte blocks if + * called to often. */ int slz_rfc1951_flush(struct slz_stream *strm, unsigned char *buf) { @@ -871,15 +874,26 @@ int slz_rfc1951_flush(struct slz_stream *strm, unsigned char *buf) send_eob(strm); } - /* send BFINAL according to state, and BTYPE=00 (lit) */ - enqueue8(strm, (strm->state == SLZ_ST_DONE) ? 1 : 0, 3); + if (strm->state == SLZ_ST_DONE) { + /* The block we've just terminated was already marked final, so + * the deflate stream is complete and nothing may be appended to + * it: an extra block here would be located past the end of the + * stream and would shift the gzip or zlib trailer that the + * caller is about to emit. Aligning the output is all we have + * to do, and it's all that's left to do for finish() as well. + */ + flush_bits(strm); + return strm->outbuf - buf; + } + + /* send BFINAL=0 and BTYPE=00 (lit) */ + enqueue8(strm, 0, 3); flush_bits(strm); // emit pending bits copy_32b(strm, 0xFFFF0000U); // len=0, nlen=~0 - /* Now the queue is empty, EOB was sent, BFINAL might have been sent if - * we completed the last block, and a zero-byte block was sent to byte- - * align the output. The last state reflects all this. Let's just - * return the number of bytes added to the output buffer. + /* Now the queue is empty, EOB was sent, and a zero-byte block was sent + * to byte-align the output. The last state reflects all this. Let's + * just return the number of bytes added to the output buffer. */ return strm->outbuf - buf; }