From: Willy Tarreau Date: Sun, 26 Jul 2026 20:05:23 +0000 (+0200) Subject: BUG/MINOR: slz: do not read past the end of the input around the match loop X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=aa72c94826929b92601b95ca8abd61108d43a175;p=thirdparty%2Fhaproxy.git BUG/MINOR: slz: do not read past the end of the input around the match loop slz_rfc1951_encode() pre-loads the first 3 bytes of the input into before entering the main loop on architectures which do not define UNALIGNED_FASTER (e.g. i386, or big endian ones). This was done unconditionally, thus inputs shorter than 3 bytes (including empty ones) caused up to 3 bytes to be read past the end of the input buffer, which may segfault if the buffer ends on the last page of a mapping. This is easily reproduced on i386 by placing a zero-length input right before an unmapped page. The pre-loaded word is only ever used if the main loop is entered, which requires at least 4 remaining bytes, so let's simply condition the load on this. The exact same case exists at the end of the loop where we can go beyond end-3 and try to read 3 or 4 bytes before getting back to the beggining of the loop, so we're using the same condition here, which helps the compiler perform the test only once and use unconditional branches from there. The code is unchanged on x86_64 and armv8 (out of ifdef) and no measurable change is observed on other archs. This should be backported to 1.2. The patch is easier consulted with git show -b. This is libslz upstream commit 4ff4b66c804629089c0bb16f141a6320f92eba10. Note: the impact in haproxy is practically inexistent since we build by default with the tag at the end of pools, even if an extra byte were to be accessed on slower architectures, it would have no effect. Better backport it to avoid triggering ASAN though. --- diff --git a/src/slz.c b/src/slz.c index 2d670f83c..08eb80de9 100644 --- a/src/slz.c +++ b/src/slz.c @@ -542,7 +542,8 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig strm->outbuf = out; #ifndef UNALIGNED_FASTER - word = ((unsigned char)in[pos] << 8) + ((unsigned char)in[pos + 1] << 16) + ((unsigned char)in[pos + 2] << 24); + if (rem >= 4) // is only used inside the loop below, hence the test for >= 4 + word = ((unsigned char)in[pos] << 8) + ((unsigned char)in[pos + 1] << 16) + ((unsigned char)in[pos + 2] << 24); #endif while (rem >= 4) { #ifndef UNALIGNED_FASTER @@ -726,11 +727,16 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig pos += mlen; #ifndef UNALIGNED_FASTER + /* same as before the loop, this is only used when continuing + * in the loop, so let's use the same condition (rem>=4). + */ + if (rem >= 4) { #ifdef UNALIGNED_LE_OK - word = *(uint32_t *)&in[pos - 1]; + word = *(uint32_t *)&in[pos - 1]; #else - word = ((unsigned char)in[pos] << 8) + ((unsigned char)in[pos + 1] << 16) + ((unsigned char)in[pos + 2] << 24); + word = ((unsigned char)in[pos] << 8) + ((unsigned char)in[pos + 1] << 16) + ((unsigned char)in[pos + 2] << 24); #endif + } #endif }