Attachment 16164 Details for Bug 14263 – Metze's patch with BUG:, s-o-b, RB+, sans TODO.

[patch] Metze's patch with BUG:, s-o-b, RB+, sans TODO.

0001-lzxpress-add-bounds-checking-to-lzxpress_decompress.patch (text/plain), 3.31 KB, created by Douglas Bagnall on 2020-08-06 22:19:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Douglas Bagnall

Created: 2020-08-06 22:19:43 UTC

Size: 3.31 KB

patch

obsolete

>From c0959568f4456e63eb35524972abb5d7533dc975 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 7 Nov 2019 10:03:36 +0100
>Subject: [PATCH] lzxpress: add bounds checking to lzxpress_decompress()
>
>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>
>---
> lib/compression/lzxpress.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
>diff --git a/lib/compression/lzxpress.c b/lib/compression/lzxpress.c
>index 024aba4c2ce..105e9c209f0 100644
>--- a/lib/compression/lzxpress.c
>+++ b/lib/compression/lzxpress.c
>@@ -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);
>-- 
>2.17.1
>

Flags:

metze: review+

Actions: View

Attachments on bug 14263: 15763 | 15764 | 15765 | 15766 | 15767 | 15768 | 15769 | 15770 | 15771 | 15772 | 15774 | 16164 | 16166