Browse code

merge security fixes

git-svn: trunk@3626

Tomasz Kojm authored on 2008/02/13 19:34:58
Showing 7 changed files
... ...
@@ -1,3 +1,16 @@
1
+Wed Feb 13 11:21:04 CET 2008 (tk)
2
+---------------------------------
3
+  * Merge security fixes:
4
+  * libclamav/mew.c: fix possible heap corruption (bb#806)
5
+    Found by Elliot, broken module disabled via daily.cvd published on Feb 2
6
+  * libclamav/pe.c: fix possible integer overflow (CVE-2008-0318)
7
+    Found by Silvio Cesare working with the VeriSign iDefense VCP;
8
+    broken module disabled via daily.cvd published on Jan 11, 2008
9
+  * libclamav/vba_extract.c: fix extraction of embedded files (bb#760)
10
+  * libclamav/cab.c: improve handling of stored files (bb#771)
11
+  * libclamav/scanners.c: respect recursion limits in cli_scanembpe() (bb#771)
12
+  * libclamav/unarj.c: improve bounds checking (bb#811)
13
+
1 14
 Wed Feb 13 12:11:08 EET 2008 (edwin)
2 15
 ------------------------------------
3 16
   * libclamav/htmlnorm.c: SVN r3619 broke phishing detection, fixed it
... ...
@@ -233,8 +233,10 @@ int cab_open(int fd, off_t offset, struct cab_archive *cab)
233 233
 
234 234
     cab->length = EC32(hdr.cbCabinet);
235 235
     cli_dbgmsg("CAB: Cabinet length: %u\n", cab->length);
236
-    if((off_t) cab->length > rsize)
236
+    if((off_t) cab->length > rsize) {
237
+	cab->length = (uint32_t) rsize;
237 238
 	bscore++;
239
+    }
238 240
 
239 241
     cab->nfolders = EC16(hdr.cFolders);
240 242
     if(!cab->nfolders) {
... ...
@@ -690,6 +692,10 @@ int cab_extract(struct cab_file *file, const char *name)
690 690
 	case 0x0000: /* STORE */
691 691
 	    cli_dbgmsg("CAB: Compression method: STORED\n");
692 692
 	    CAB_CHGFOLDER;
693
+	    if(file->length > file->cab->length) {
694
+		cli_dbgmsg("cab_extract: Stored file larger than archive itself, trimming down\n");
695
+		file->length = file->cab->length;
696
+	    }
693 697
 	    ret = cab_unstore(file, file->length);
694 698
 	    break;
695 699
 
... ...
@@ -784,13 +784,15 @@ int unmew11(char *src, int off, int ssize, int dsize, uint32_t base, uint32_t va
784 784
 	entry_point  = cli_readint32(source + 4);
785 785
 	newedi = cli_readint32(source + 8);
786 786
 	ledi = src + (newedi - vma);
787
+	loc_ds = size_sum - (newedi - vma);
787 788
 
788 789
 	i = 0;
789
-	ssize -= 12;
790
+	loc_ss -= 12;
791
+	loc_ss -= off;
790 792
 	while (1)
791 793
 	{
792 794
   		cli_dbgmsg("MEW unpacking section %d (%p->%p)\n", i, lesi, ledi);
793
-		if (!CLI_ISCONTAINED(src, size_sum, lesi, 4) || !CLI_ISCONTAINED(src, size_sum, ledi, 4))
795
+		if (!CLI_ISCONTAINED(src, size_sum, lesi, loc_ss) || !CLI_ISCONTAINED(src, size_sum, ledi, loc_ds))
794 796
 		{
795 797
 			cli_dbgmsg("Possibly programmer error or hand-crafted PE file, report to clamav team\n");
796 798
 			return -1;
... ...
@@ -810,10 +812,11 @@ int unmew11(char *src, int off, int ssize, int dsize, uint32_t base, uint32_t va
810 810
 
811 811
 		/* XXX */
812 812
 		loc_ss -= (f1+4-lesi);
813
-		loc_ds -= (f2-ledi);
814
-		ledi = src + (cli_readint32(f1) - vma);
815 813
 		lesi = f1+4;
816 814
 
815
+		ledi = src + (cli_readint32(f1) - vma);
816
+		loc_ds = size_sum - (cli_readint32(f1) - vma);
817
+
817 818
 		if (!uselzma)
818 819
 		{
819 820
 			uint32_t val = PESALIGN(f2 - src, 0x1000);
... ...
@@ -811,6 +811,18 @@ int cli_scanpe(int desc, cli_ctx *ctx)
811 811
 	    }
812 812
 	}
813 813
 
814
+	if (exe_sections[i].urva>>31 || exe_sections[i].uvsz>>31 || (exe_sections[i].rsz && exe_sections[i].uraw>>31) || exe_sections[i].ursz>>31) {
815
+	    cli_dbgmsg("Found PE values with sign bit set\n");
816
+	    free(section_hdr);
817
+	    free(exe_sections);
818
+	    if(DETECT_BROKEN) {
819
+	        if(ctx->virname)
820
+		    *ctx->virname = "Broken.Executable";
821
+		return CL_VIRUS;
822
+	    }
823
+	    return CL_CLEAN;
824
+	}
825
+
814 826
 	if(!i) {
815 827
 	    if (DETECT_BROKEN && exe_sections[i].urva!=hdr_size) { /* Bad first section RVA */
816 828
 	        cli_dbgmsg("First section is in the wrong place\n");
... ...
@@ -1533,6 +1533,7 @@ static int cli_scanembpe(int desc, cli_ctx *ctx)
1533 1533
 	return CL_EFSYNC;
1534 1534
     }
1535 1535
 
1536
+    ctx->recursion++;
1536 1537
     lseek(fd, 0, SEEK_SET);
1537 1538
     if((ret = cli_magic_scandesc(fd, ctx)) == CL_VIRUS) {
1538 1539
 	cli_dbgmsg("cli_scanembpe: Infected with %s\n", *ctx->virname);
... ...
@@ -1542,6 +1543,7 @@ static int cli_scanembpe(int desc, cli_ctx *ctx)
1542 1542
 	free(tmpname);	
1543 1543
 	return CL_VIRUS;
1544 1544
     }
1545
+    ctx->recursion--;
1545 1546
 
1546 1547
     close(fd);
1547 1548
     if(!cli_leavetemps_flag)
... ...
@@ -226,6 +226,10 @@ static int make_table(arj_decode_t *decode_data, int nchar, unsigned char *bitle
226 226
 		count[i] = 0;
227 227
 	}
228 228
 	for (i = 0; (int)i < nchar; i++) {
229
+		if (bitlen[i] >= 17) {
230
+			cli_dbgmsg("UNARJ: bounds exceeded\n");
231
+			return CL_EARJ;
232
+		}
229 233
 		count[bitlen[i]]++;
230 234
 	}
231 235
 	
... ...
@@ -238,6 +242,10 @@ static int make_table(arj_decode_t *decode_data, int nchar, unsigned char *bitle
238 238
 	}
239 239
 	
240 240
 	jutbits = 16 - tablebits;
241
+	if (tablebits >= 17) {
242
+		cli_dbgmsg("UNARJ: bounds exceeded\n");
243
+		return CL_EARJ;
244
+	}
241 245
 	for (i = 1; (int)i <= tablebits; i++) {
242 246
 		start[i] >>= jutbits;
243 247
 		weight[i] = 1 << (tablebits - i);
... ...
@@ -251,6 +259,10 @@ static int make_table(arj_decode_t *decode_data, int nchar, unsigned char *bitle
251 251
 	if (i != (unsigned short) (1 << 16)) {
252 252
 		k = 1 << tablebits;
253 253
 		while (i != k) {
254
+			if (i >= tablesize) {
255
+				cli_dbgmsg("UNARJ: bounds exceeded\n");
256
+				return CL_EARJ;
257
+			}
254 258
 			table[i++] = 0;
255 259
 		}
256 260
 	}
... ...
@@ -261,6 +273,10 @@ static int make_table(arj_decode_t *decode_data, int nchar, unsigned char *bitle
261 261
 		if ((len = bitlen[ch]) == 0) {
262 262
 			continue;
263 263
 		}
264
+		if (len >= 17) {
265
+			cli_dbgmsg("UNARJ: bounds exceeded\n");
266
+			return CL_EARJ;
267
+		}
264 268
 		k = start[len];
265 269
 		nextcode = k + weight[len];
266 270
 		if ((int)len <= tablebits) {
... ...
@@ -275,9 +291,17 @@ static int make_table(arj_decode_t *decode_data, int nchar, unsigned char *bitle
275 275
 			i = len - tablebits;
276 276
 			while (i != 0) {
277 277
 				if (*p == 0) {
278
+					if (avail >= (2 * NC - 1)) {
279
+						cli_dbgmsg("UNARJ: bounds exceeded\n");
280
+						return CL_EARJ;
281
+					}
278 282
 					decode_data->right[avail] = decode_data->left[avail] = 0;
279 283
 					*p = avail++;
280 284
 				}
285
+				if (*p >= (2 * NC - 1)) {
286
+					cli_dbgmsg("UNARJ: bounds exceeded\n");
287
+					return CL_EARJ;
288
+				}
281 289
 				if (k & mask) {
282 290
 					p = &decode_data->right[*p];
283 291
 				} else {
... ...
@@ -301,6 +325,10 @@ static void read_pt_len(arj_decode_t *decode_data, int nn, int nbit, int i_speci
301 301
 	
302 302
 	n = arj_getbits(decode_data, nbit);
303 303
 	if (n == 0) {
304
+		if (nn > NPT) {
305
+			cli_dbgmsg("UNARJ: bounds exceeded\n");
306
+			return;
307
+		}
304 308
 		c = arj_getbits(decode_data, nbit);
305 309
 		for (i = 0; i < nn; i++) {
306 310
 			decode_data->pt_len[i] = 0;
... ...
@@ -368,6 +396,10 @@ static int read_c_len(arj_decode_t *decode_data)
368 368
 					mask >>= 1;
369 369
 				} while (c >= NT);
370 370
 			}
371
+			if (c >= 19) {
372
+				cli_dbgmsg("UNARJ: bounds exceeded\n");
373
+				return CL_EARJ;
374
+			}
371 375
 			fill_buf(decode_data, (int)(decode_data->pt_len[c]));
372 376
 			if (c <= 2) {
373 377
 				if (c == 0) {
... ...
@@ -752,7 +752,7 @@ ppt_unlzw(const char *dir, int fd, uint32_t length)
752 752
 		}
753 753
 	} while(inflate(&stream, Z_NO_FLUSH) == Z_OK);
754 754
 
755
-	if (cli_writen(ofd, outbuff, bufflen) != (int)bufflen) {
755
+	if (cli_writen(ofd, outbuff, PPT_LZW_BUFFSIZE-stream.avail_out) != (int)PPT_LZW_BUFFSIZE-stream.avail_out) {
756 756
 		close(ofd);
757 757
 		inflateEnd(&stream);
758 758
 		return FALSE;