Apply proposed changes to fix RAR VMSF_DELTA Filter Signedness error (CVE-2012-6706) Cherry picked from commit a7d8447bd9a4d5ae1fa970c1849c8caeb5f1a805 [Link 1] and d4699442bce76574573dc564e7f2177d679b88bd [Link 2]. Link 1: https://github.com/Cisco-Talos/clamav-devel/commit/a7d8447bd9a4d5ae1fa970c1849c8caeb5f1a805 Link 2: https://github.com/Cisco-Talos/clamav-devel/commit/d4699442bce76574573dc564e7f2177d679b88bd --- a/libclamunrar/unrarvm.c +++ b/libclamunrar/unrarvm.c @@ -213,17 +213,20 @@ void rarvm_addbits(rarvm_input_t *rarvm_input, int bits) unsigned int rarvm_getbits(rarvm_input_t *rarvm_input) { - unsigned int bit_field; + unsigned int bit_field = 0; - if (rarvm_input->in_addr+2 < rarvm_input->buf_size) { + if (rarvm_input->in_addr < rarvm_input->buf_size) { bit_field = (unsigned int) rarvm_input->in_buf[rarvm_input->in_addr] << 16; - bit_field |= (unsigned int) rarvm_input->in_buf[rarvm_input->in_addr+1] << 8; - bit_field |= (unsigned int) rarvm_input->in_buf[rarvm_input->in_addr+2]; - bit_field >>= (8-rarvm_input->in_bit); - - return (bit_field & 0xffff); + if (rarvm_input->in_addr+1 < rarvm_input->buf_size) { + bit_field |= (unsigned int) rarvm_input->in_buf[rarvm_input->in_addr+1] << 8; + if (rarvm_input->in_addr+2 < rarvm_input->buf_size) { + bit_field |= (unsigned int) rarvm_input->in_buf[rarvm_input->in_addr+2]; + } + } } - return 0; + bit_field >>= (8-rarvm_input->in_bit); + + return (bit_field & 0xffff); } unsigned int rarvm_read_data(rarvm_input_t *rarvm_input) @@ -311,10 +314,10 @@ static unsigned int *rarvm_get_operand(rarvm_data_t *rarvm_data, } } -static unsigned int filter_itanium_getbits(unsigned char *data, int bit_pos, int bit_count) +static unsigned int filter_itanium_getbits(unsigned char *data, unsigned int bit_pos, unsigned int bit_count) { - int in_addr=bit_pos/8; - int in_bit=bit_pos&7; + unsigned int in_addr=bit_pos/8; + unsigned int in_bit=bit_pos&7; unsigned int bit_field=(unsigned int)data[in_addr++]; bit_field|=(unsigned int)data[in_addr++] << 8; bit_field|=(unsigned int)data[in_addr++] << 16; @@ -323,10 +326,10 @@ static unsigned int filter_itanium_getbits(unsigned char *data, int bit_pos, int return(bit_field & (0xffffffff>>(32-bit_count))); } -static void filter_itanium_setbits(unsigned char *data, unsigned int bit_field, int bit_pos, int bit_count) +static void filter_itanium_setbits(unsigned char *data, unsigned int bit_field, unsigned int bit_pos, unsigned int bit_count) { - int i, in_addr=bit_pos/8; - int in_bit=bit_pos&7; + unsigned int i, in_addr=bit_pos/8; + unsigned int in_bit=bit_pos&7; unsigned int and_mask=0xffffffff>>(32-bit_count); and_mask=~(and_mask<R[4]; file_offset = rarvm_data->R[6]; - if (((unsigned int)data_size >= VM_GLOBALMEMADDR) || (data_size < 4)) { + if ((data_size > VM_GLOBALMEMADDR) || (data_size < 4)) { break; } @@ -367,12 +371,14 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil if (cur_byte==0xe8 || cur_byte==cmp_byte2) { offset = cur_pos+file_offset; addr = GET_VALUE(FALSE, data); - if (addr < 0) { - if (addr+offset >=0 ) { + // We check 0x80000000 bit instead of '< 0' comparison + // not assuming int32 presence or uint size and endianness. + if ((addr & 0x80000000)!=0) { // addr<0 + if (((addr+offset) & 0x80000000)==0) { // addr+offset>=0 SET_VALUE(FALSE, data, addr+file_size); } } else { - if (addrR[4]; file_offset = rarvm_data->R[6]; - if (((unsigned int)data_size >= VM_GLOBALMEMADDR) || (data_size < 21)) { + if ((data_size > VM_GLOBALMEMADDR) || (data_size < 21)) { break; } @@ -429,7 +435,7 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil border = data_size*2; SET_VALUE(FALSE, &rarvm_data->mem[VM_GLOBALMEMADDR+0x20], data_size); - if ((unsigned int)data_size >= VM_GLOBALMEMADDR/2) { + if (data_size > VM_GLOBALMEMADDR/2 || channels > 1024 || channels == 0) { break; } for (cur_channel=0 ; cur_channel < channels ; cur_channel++) { @@ -440,7 +446,7 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil } break; case VMSF_RGB: { - const int channels=3; + const unsigned int channels=3; data_size = rarvm_data->R[4]; width = rarvm_data->R[0] - 3; PosR = rarvm_data->R[1]; @@ -448,15 +454,14 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil dest_data = src_data + data_size; SET_VALUE(FALSE, &rarvm_data->mem[VM_GLOBALMEMADDR+0x20], data_size); - if ((unsigned int)data_size >= VM_GLOBALMEMADDR/2) { + if (data_size > VM_GLOBALMEMADDR/2 || data_size < 3 || width > data_size || PosR > 2) { break; } for (cur_channel=0 ; cur_channel < channels; cur_channel++) { unsigned int prev_byte = 0; for (i=cur_channel ; i= 3) { - unsigned char *upper_data = dest_data+upper_pos; + if (i >= width+3) { + unsigned char *upper_data = dest_data+i-width; unsigned int upper_byte = *upper_data; unsigned int upper_left_byte = *(upper_data-3); predicted = prev_byte+upper_byte-upper_left_byte; @@ -486,13 +491,14 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil break; } case VMSF_AUDIO: { - int channels=rarvm_data->R[0]; + unsigned int channels=rarvm_data->R[0]; data_size = rarvm_data->R[4]; src_data = rarvm_data->mem; dest_data = src_data + data_size; SET_VALUE(FALSE, &rarvm_data->mem[VM_GLOBALMEMADDR+0x20], data_size); - if ((unsigned int)data_size >= VM_GLOBALMEMADDR/2) { + // In fact, audio channels never exceed 4. + if (data_size > VM_GLOBALMEMADDR/2 || channels > 128 || channels == 0) { break; } for (cur_channel=0 ; cur_channel < channels ; cur_channel++) { @@ -553,7 +559,7 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil data_size = rarvm_data->R[4]; src_pos = 0; dest_pos = data_size; - if ((unsigned int)data_size >= VM_GLOBALMEMADDR/2) { + if (data_size > VM_GLOBALMEMADDR/2) { break; } while (src_pos < data_size) { -- 2.16.2