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 <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/lib/compression/lzxpress_huffman.c b/lib/compression/lzxpress_huffman.c
index 990552a7d09d62c7e44c36f49e3e45496b462671..ee3eb272fc039a932806baf9f4ab59aec6ea6b9c 100644 (file)
--- 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;
                }