pkgsrc-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

security/clamav: patch for CVE-2012-6706 (unrar arbitrary memory overwrite)



Hi,

I took this patch from github and Debian:

https://github.com/vrtadmin/clamav-devel/commit/d4699442bce76574573dc564e7f2177d679b88bd
(via https://security-tracker.debian.org/tracker/CVE-2012-6706,
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867223)

I wonder why there is no fix upstream yet at clamav.net.

Only compile-tested (and the binary does start).

Sorry that I don't provide all the checksums etc., I am quite in a hurry
atm.

Regards
Matthias Ferdinand
$NetBSD$

Patch against CVE-2012-6706 (unrar arbitrary memory write),
taken almost verbatim from 
https://github.com/vrtadmin/clamav-devel/commit/d4699442bce76574573dc564e7f2177d679b88bd
(via https://security-tracker.debian.org/tracker/CVE-2012-6706,
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867223)

From f4b4e08b9420088f9d385498d716f0e0384bf2fc Mon Sep 17 00:00:00 2001
From: Mickey Sola <msola%sourcefire.com@localhost>
Date: Thu, 29 Jun 2017 14:02:03 -0400
Subject: unrar - adding proposed changes to fix RAR VMSF_DELTA Filter
 Signedness error

CVE: CVE-2012-6706: arbitrary memory write
BTS: #867223
Patch-Name: unrar-adding-proposed-changes-to-fix-RAR-VMSF_DELTA-.patch
---
 libclamunrar/unrarvm.c | 55 ++++++++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/libclamunrar/unrarvm.c b/libclamunrar/unrarvm.c
index 102fe2ebf044..b21e242fa72b 100644
--- libclamunrar/unrarvm.c
+++ libclamunrar/unrarvm.c
@@ -314,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;
@@ -326,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<<in_bit);
 
@@ -346,11 +346,12 @@ static void filter_itanium_setbits(unsigned char *data, unsigned int bit_field,
 static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_filters_t filter_type)
 {
 	unsigned char *data, cmp_byte2, cur_byte, *src_data, *dest_data;
-	int i, j, data_size, channels, src_pos, dest_pos, border, width, PosR;
-	int op_type, cur_channel, byte_count, start_pos, pa, pb, pc;
+	unsigned int i, j, data_size, channels, src_pos, dest_pos, border, width, PosR;
+	unsigned int op_type, cur_channel, byte_count, start_pos;
+	int pa, pb, pc;
 	unsigned int file_offset, cur_pos, predicted;
-	int32_t offset, addr;
-	const int file_size=0x1000000;
+	uint32_t offset, addr;
+	const unsigned int file_size=0x1000000;
 
 	switch(filter_type) {
 	case VMSF_E8:
@@ -359,7 +360,7 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil
 		data_size = rarvm_data->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;
 		}
 
@@ -370,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 (addr<file_size) {
+					if (((addr-file_size) & 0x80000000)!=0) { // addr<file_size
 						SET_VALUE(FALSE, data, addr-offset);
 					}
 				}
@@ -389,7 +392,7 @@ static void execute_standard_filter(rarvm_data_t *rarvm_data, rarvm_standard_fil
 		data_size = rarvm_data->R[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;
 		}
 		
@@ -432,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++) {
@@ -443,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];
@@ -451,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<data_size ; i+=channels) {
-				int upper_pos=i-width;
-				if (upper_pos >= 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;
@@ -489,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++) {
@@ -556,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) {


Home | Main Index | Thread Index | Old Index