lzxpress: add bounds checking to lzxpress_decompress()
authorStefan Metzmacher <metze@samba.org>
Thu, 7 Nov 2019 09:03:36 +0000 (10:03 +0100)
committerDouglas Bagnall <dbagnall@samba.org>
Sun, 9 Aug 2020 00:30:26 +0000 (00:30 +0000)
lzxpress_decompress() would wander past the end of the array in
numerous locations.

Credit to OSS-Fuzz.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14190
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19382
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20083
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22485
REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22667

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Sun Aug  9 00:30:26 UTC 2020 on sn-devel-184

lib/compression/lzxpress.c

index 024aba4c2ce8aa1b868a1001c36cc5091cb894a0..d8326304455cbd1d05151c9b83930b5de343a09c 100644 (file)
@@ -252,8 +252,24 @@ ssize_t lzxpress_decompress(const uint8_t *input,
        offset = 0;
        nibble_index = 0;
 
+#define __CHECK_BYTES(__size, __index, __needed) do { \
+       if (unlikely(__index >= __size)) { \
+               return -1; \
+       } else { \
+               uint32_t __avail = __size - __index; \
+               if (unlikely(__needed > __avail)) { \
+                       return -1; \
+               } \
+       } \
+} while(0)
+#define CHECK_INPUT_BYTES(__needed) \
+       __CHECK_BYTES(input_size, input_index, __needed)
+#define CHECK_OUTPUT_BYTES(__needed) \
+       __CHECK_BYTES(max_output_size, output_index, __needed)
+
        do {
                if (indicator_bit == 0) {
+                       CHECK_INPUT_BYTES(4);
                        indicator = PULL_LE_UINT32(input, input_index);
                        input_index += sizeof(uint32_t);
                        indicator_bit = 32;
@@ -266,10 +282,13 @@ ssize_t lzxpress_decompress(const uint8_t *input,
                 * check whether the 4th bit of the value in indicator is set
                 */
                if (((indicator >> indicator_bit) & 1) == 0) {
+                       CHECK_INPUT_BYTES(1);
+                       CHECK_OUTPUT_BYTES(1);
                        output[output_index] = input[input_index];
                        input_index += sizeof(uint8_t);
                        output_index += sizeof(uint8_t);
                } else {
+                       CHECK_INPUT_BYTES(2);
                        length = PULL_LE_UINT16(input, input_index);
                        input_index += sizeof(uint16_t);
                        offset = length / 8;
@@ -277,6 +296,7 @@ ssize_t lzxpress_decompress(const uint8_t *input,
 
                        if (length == 7) {
                                if (nibble_index == 0) {
+                                       CHECK_INPUT_BYTES(1);
                                        nibble_index = input_index;
                                        length = input[input_index] % 16;
                                        input_index += sizeof(uint8_t);
@@ -286,9 +306,11 @@ ssize_t lzxpress_decompress(const uint8_t *input,
                                }
 
                                if (length == 15) {
+                                       CHECK_INPUT_BYTES(1);
                                        length = input[input_index];
                                        input_index += sizeof(uint8_t);
                                        if (length == 255) {
+                                               CHECK_INPUT_BYTES(2);
                                                length = PULL_LE_UINT16(input, input_index);
                                                input_index += sizeof(uint16_t);
                                                length -= (15 + 7);
@@ -299,10 +321,16 @@ ssize_t lzxpress_decompress(const uint8_t *input,
                        }
 
                        length += 3;
+                       if (length == 0) {
+                               return -1;
+                       }
 
-                       do {
-                               if ((output_index >= max_output_size) || ((offset + 1) > output_index)) break;
+                       if (offset >= output_index) {
+                               return -1;
+                       }
+                       CHECK_OUTPUT_BYTES(length);
 
+                       do {
                                output[output_index] = output[output_index - offset - 1];
 
                                output_index += sizeof(uint8_t);