From: Douglas Bagnall Date: Sat, 3 Dec 2022 22:33:29 +0000 (+1300) Subject: compression/huffman: avoid semi-defined behaviour in decompress X-Git-Tag: talloc-2.4.0~207 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f77b376d470dd318f0a9699b3528018ce8ea49a;p=thirdparty%2Fsamba.git compression/huffman: avoid semi-defined behaviour in decompress We had output[output_pos - distance]; where output_pos and distance are size_t and distance can be greater than output_pos (because it refers to a place in the previous block). The underflow is defined, leading to a big number, and when sizeof(size_t) == sizeof(*uint8_t) the subsequent overflow works as expected. But if size_t is smaller than a pointer, bad things will happen. This was found by OSSFuzz with 'UBSAN_OPTIONS=print_stacktrace=1:silence_unsigned_overflow=1'. Credit to OSSFuzz. Signed-off-by: Douglas Bagnall Reviewed-by: Volker Lendecke Reviewed-by: Jeremy Allison --- diff --git a/lib/compression/lzxpress_huffman.c b/lib/compression/lzxpress_huffman.c index 990552a7d09..ee3eb272fc0 100644 --- a/lib/compression/lzxpress_huffman.c +++ b/lib/compression/lzxpress_huffman.c @@ -1788,17 +1788,19 @@ static ssize_t lzx_huffman_decompress_block(struct bitstream *input, * the end of the expected block. That's fine, so long * as it doesn't extend past the total output size. */ + size_t i; size_t end = output_pos + length; + uint8_t *here = output + output_pos; + uint8_t *there = here - distance; if (end > output_size || previous_size + output_pos < distance || - unlikely(end < output_pos)) { + unlikely(end < output_pos || there > here)) { return LZXPRESS_ERROR; } - - for (; output_pos < end; output_pos++) { - output[output_pos] = \ - output[output_pos - distance]; + for (i = 0; i < length; i++) { + here[i] = there[i]; } + output_pos += length; distance = 0; length = 0; }