From 69bb9bcf6cef43b744e5baa101a8a3a78db0b00d Mon Sep 17 00:00:00 2001 From: Grzegorz Antoniak Date: Fri, 3 May 2019 08:31:28 +0200 Subject: [PATCH] RAR5 reader: fix invalid type used for dictionary size mask. This commit fixes places where the window_mask variable, which is needed to perform operations on the dictionary circular buffer, was casted to an int variable. In files that declare dictionary buffer size of 4GB, window_mask has a value of 0xFFFFFFFF. If this value will be assigned to an int variable, this will effectively make the variable to contain value of -1. This means, that any cast to a 64-bit value will bit-extend the int variable to 0xFFFFFFFFFFFFFFFF. This was happening during a read operation from the dictionary. Such invalid window_mask variable was not guarding against buffer underflow. This commit should fix the OSSFuzz issue #14537. The commit also contains a test case for this issue. --- libarchive/archive_read_support_format_rar5.c | 22 +++++++++---------- libarchive/test/test_read_format_rar5.c | 15 +++++++++++++ ..._format_rar5_invalid_dict_reference.rar.uu | 9 ++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index e3c3b62d1..9f6dc6437 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -505,7 +505,7 @@ static void write_filter_data(struct rar5* rar, uint32_t offset, archive_le32enc(&rar->cstate.filtered_buf[offset], value); } -static void circular_memcpy(uint8_t* dst, uint8_t* window, const int mask, +static void circular_memcpy(uint8_t* dst, uint8_t* window, const uint64_t mask, int64_t start, int64_t end) { if((start & mask) > (end & mask)) { @@ -562,16 +562,15 @@ static int run_e8e9_filter(struct rar5* rar, struct filter_info* flt, const uint32_t file_size = 0x1000000; ssize_t i; - const int mask = (int)rar->cstate.window_mask; circular_memcpy(rar->cstate.filtered_buf, rar->cstate.window_buf, - mask, + rar->cstate.window_mask, rar->cstate.solid_offset + flt->block_start, rar->cstate.solid_offset + flt->block_start + flt->block_length); for(i = 0; i < flt->block_length - 4;) { uint8_t b = rar->cstate.window_buf[(rar->cstate.solid_offset + - flt->block_start + i++) & mask]; + flt->block_start + i++) & rar->cstate.window_mask]; /* 0xE8 = x86's call (function call) * 0xE9 = x86's jmp (unconditional jump) */ @@ -604,22 +603,21 @@ static int run_e8e9_filter(struct rar5* rar, struct filter_info* flt, static int run_arm_filter(struct rar5* rar, struct filter_info* flt) { ssize_t i = 0; uint32_t offset; - const int mask = (int)rar->cstate.window_mask; circular_memcpy(rar->cstate.filtered_buf, rar->cstate.window_buf, - mask, + rar->cstate.window_mask, rar->cstate.solid_offset + flt->block_start, rar->cstate.solid_offset + flt->block_start + flt->block_length); for(i = 0; i < flt->block_length - 3; i += 4) { uint8_t* b = &rar->cstate.window_buf[(rar->cstate.solid_offset + - flt->block_start + i) & mask]; + flt->block_start + i) & rar->cstate.window_mask]; if(b[3] == 0xEB) { /* 0xEB = ARM's BL (branch + link) instruction. */ offset = read_filter_data(rar, (rar->cstate.solid_offset + - flt->block_start + i) & mask) & 0x00ffffff; + flt->block_start + i) & rar->cstate.window_mask) & 0x00ffffff; offset -= (uint32_t) ((i + flt->block_start) / 4); offset = (offset & 0x00ffffff) | 0xeb000000; @@ -689,7 +687,7 @@ static int run_filter(struct archive_read* a, struct filter_info* flt) { static void push_data(struct archive_read* a, struct rar5* rar, const uint8_t* buf, int64_t idx_begin, int64_t idx_end) { - const int wmask = (int)rar->cstate.window_mask; + const uint64_t wmask = rar->cstate.window_mask; const ssize_t solid_write_ptr = (rar->cstate.solid_offset + rar->cstate.last_write_ptr) & wmask; @@ -2660,8 +2658,8 @@ static int decode_code_length(struct rar5* rar, const uint8_t* p, static int copy_string(struct archive_read* a, int len, int dist) { struct rar5* rar = get_context(a); - const int cmask = (int)rar->cstate.window_mask; - const int64_t write_ptr = rar->cstate.write_ptr + rar->cstate.solid_offset; + const uint64_t cmask = rar->cstate.window_mask; + const uint64_t write_ptr = rar->cstate.write_ptr + rar->cstate.solid_offset; int i; /* The unpacker spends most of the time in this function. It would be @@ -2686,7 +2684,7 @@ static int do_uncompress_block(struct archive_read* a, const uint8_t* p) { uint16_t num; int ret; - const int cmask = (int)rar->cstate.window_mask; + const uint64_t cmask = rar->cstate.window_mask; const struct compressed_block_header* hdr = &rar->last_block_hdr; const uint8_t bit_size = 1 + bf_bit_size(hdr); diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c index cc1c4b896..e53cf061a 100644 --- a/libarchive/test/test_read_format_rar5.c +++ b/libarchive/test/test_read_format_rar5.c @@ -1010,3 +1010,18 @@ DEFINE_TEST(test_read_format_rar5_truncated_huff) EPILOGUE(); } + +DEFINE_TEST(test_read_format_rar5_invalid_dict_reference) +{ + uint8_t buf[16]; + + PROLOGUE("test_read_format_rar5_invalid_dict_reference.rar"); + + assertA(0 == archive_read_next_header(a, &ae)); + /* This archive is invalid. However, processing it shouldn't cause any + * errors related to buffer underflow when using -fsanitize. */ + assertA(ARCHIVE_FATAL == archive_read_data(a, buf, sizeof(buf))); + assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae)); + + EPILOGUE(); +} \ No newline at end of file diff --git a/libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu b/libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu new file mode 100644 index 000000000..9b78c9b36 --- /dev/null +++ b/libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu @@ -0,0 +1,9 @@ +begin 644 test_read_format_rar5_invalid_dict_reference.rar +M4F%R(1H'`0"-[P+2``+#!QR`!P`F^P#_^_O[^_O[^R4``B$<`0(`#@```0`` +M`"#2````_____QH(`/__^P#_W5)04(#_`(:&;;%DS+?,L0```````````+%D +MS+*RLK*R/@``____Y`"R````XP```````!4``````.X````````````````` +M%52$A(2$A(2$A(2$A(2$A(2$Q(2$A(2$A(2$A(2)]( +` +end -- 2.47.2