Attachment #43076 for bug #69150

View | Details | Raw Unified | Return to bug 69150
Collapse All | Expand All




2004-10-31  Russell Marks  <russell.marks@ntlworld.com>

	* Added width/height limits to all picture readers, 32767x32767 is
	now the maximum image size supported (consistent with xzgv). This
	is a crude (albeit effective) fix for heap overflow bugs - there
	may yet be more subtle problems, but I can't really fix them until
	I know they're there. :-) Thanks to Luke Macken for letting me
	know about the heap overflow problems. I suppose I should also
	thank "infamous41md" for publishing the original exploit (for the
	XPM colours bug), even if he didn't bother emailing me or
	anything.

	* src/readxpm.c (read_xpm_file): fix for exploitable malloc() arg
	overflow. There are several more of these in zgv, but this is the
	easiest to fix.

2004-07-08  Russell Marks  <russell.marks@ntlworld.com>

	* src/readgif.c (read_gif_file): added more multiple-image (e.g.
	animated) GIF brokenness checks than before. Previously it was
	possible to get a segfault with the `right' file, despite there
	already being various range checks. Thanks to Mikulas Patocka for
	spotting this.

2004-03-29  Russell Marks  <russell.marks@ntlworld.com>

	* Version 5.8.

Lines 177-183 Link Here

(-)zgv-5.8/src/readbmp.c (-1 / +2 lines)
177	bytepp=1;	177	bytepp=1;
178	if ((pp->bpp == 24) && (*output_type == 3))	178	if ((pp->bpp == 24) && (*output_type == 3))
179	bytepp = 3;	179	bytepp = 3;
180	if ((work_bmap = bmap = calloc (w (h + 2) * bytepp,1)) == NULL)	180	if (WH_BAD(w,h) \|\|
		181	(work_bmap = bmap = calloc (w (h + 2) * bytepp,1)) == NULL)
181	CLOSE_AND_RET(_PICERR_NOMEM);	182	CLOSE_AND_RET(_PICERR_NOMEM);
182		183
183	bytes_in_image=whbytepp;	184	bytes_in_image=whbytepp;

Lines 491-497 Link Here

(-)zgv-5.8/src/readgif.c (-15 / +28 lines)
491	readcolmap(in,im->cmap,lnumcols);	491	readcolmap(in,im->cmap,lnumcols);
492	}	492	}
493		493
494	if((im->image=(byte )malloc(widthheight))==NULL)	494	if(WH_BAD(width,height) \|\| (im->image=(byte )malloc(widthheight))==NULL)
495	{	495	{
496	fclose(in);	496	fclose(in);
497	return(_PICERR_NOMEM);	497	return(_PICERR_NOMEM);
Lines 599-605 Link Here
599		599
600	/* allocate main image and palette */	600	/* allocate main image and palette */
601		601
602	if((theimageptr=(byte )malloc(ginfo->width*ginfo->height))==NULL)	602	if(WH_BAD(ginfo->width,ginfo->height) \|\|
		603	(theimageptr=(byte )malloc(ginfo->width*ginfo->height))==NULL)
603	{	604	{
604	images_cleanup();	605	images_cleanup();
605	return(_PICERR_NOMEM);	606	return(_PICERR_NOMEM);
Lines 668-674 Link Here
668	for(i=0;i<imagecount;i++)	669	for(i=0;i<imagecount;i++)
669	{	670	{
670	int x,y,left,w;	671	int x,y,left,w;
671	unsigned char ptr1,ptr2;	672	unsigned char ptr1,ptr2,*oldptr1;
		673
		674	/* basic width/height vs. "screen" checks, left/top handled elsewhere */
		675	if(images[i]->width>swidth) images[i]->width=swidth;
		676	if(images[i]->height>sheight) images[i]->height=sheight;
672		677
673	/* for images after the first, we need to set the initial contents	678	/* for images after the first, we need to set the initial contents
674	* (as far as GIF is concerned, the `screen' contents) as directed	679	* (as far as GIF is concerned, the `screen' contents) as directed
Lines 708-727 Link Here
708	*/	713	*/
709	}	714	}
710	}	715	}
711		716
712	ptr1=ptr+images[i]->left+images[i]->top*swidth;	717	/* an image with left or top offscreen is broken, but relying
713	ptr2=images[i]->image;	718	* unknowingly on the image not appearing at all. So skip it.
714		719	*/
715	for(y=0;y<images[i]->height;y++)	720	if(images[i]->left<swidth && images[i]->top<sheight)
716	{	721	{
717	for(x=0;x<images[i]->width;x++)	722	ptr1=ptr+images[i]->left+images[i]->top*swidth;
718	if(!(images[i]->gcb_control&1) \|\| /* if no transparent col defined */
719	images[i]->transparent_col!=*ptr2)
720	ptr1++=ptr2++;
721	else
722	ptr1++,ptr2++;
723		723
724	ptr1+=swidth-images[i]->width;	724	for(y=0;y<images[i]->height && images[i]->top+y<sheight;y++)
		725	{
		726	oldptr1=ptr1;
		727	ptr2=images[i]->image+y*images[i]->width;
		728
		729	for(x=0;x<images[i]->width && images[i]->left+x<swidth;x++)
		730	if(!(images[i]->gcb_control&1) \|\| /* if no transparent col defined */
		731	images[i]->transparent_col!=*ptr2)
		732	ptr1++=ptr2++;
		733	else
		734	ptr1++,ptr2++;
		735
		736	ptr1=oldptr1+swidth;
		737	}
725	}	738	}
726		739
727	ptr+=swidth*sheight;	740	ptr+=swidth*sheight;

Lines 190-199 Link Here

(-)zgv-5.8/src/readjpeg.c (-3 / +3 lines)
190	height=cinfo.output_height;	190	height=cinfo.output_height;
191	}	191	}
192		192
193	theimage=(byte )malloc(pixelsizewidth*height);	193	if(WH_BAD(width,height) \|\|
194	if(theimage==NULL)	194	(theimage=(byte )malloc(pixelsizewidth*height))==NULL)
195	{	195	{
196	jpegerr("Out of memory");	196	jpegerr("Out of memory"); /* XXX misleading if width/height are bad */
197	longjmp(jerr.setjmp_buffer,1);	197	longjmp(jerr.setjmp_buffer,1);
198	}	198	}
199		199

Lines 103-109 Link Here

(-)zgv-5.8/src/readmrf.c (-1 / +2 lines)
103	w64=(w+63)/64;	103	w64=(w+63)/64;
104	h64=(h+63)/64;	104	h64=(h+63)/64;
105		105
106	if((bmap=malloc(wh))==NULL \|\|	106	if(WH_BAD(w6464,h6464) \|\| WH_BAD(w,h) \|\|
		107	(bmap=malloc(wh))==NULL \|\|
107	(image=calloc(w64h6464*64,1))==NULL)	108	(image=calloc(w64h6464*64,1))==NULL)
108	CLOSE_AND_RET(_PICERR_NOMEM);	109	CLOSE_AND_RET(_PICERR_NOMEM);
109		110

Lines 39-45 Link Here

(-)zgv-5.8/src/readpcd.c (-1 / +1 lines)
39		39
40	if((output_type)!=1)output_type=3;	40	if((output_type)!=1)output_type=3;
41		41
42	if((bmap=malloc(w(h+3-output_type)(*output_type)))==NULL)	42	if(WH_BAD(w,h) \|\| (bmap=malloc(w(h+3-output_type)(*output_type)))==NULL)
43	return(_PICERR_NOMEM);	43	return(_PICERR_NOMEM);
44		44
45	if((*pal=malloc(768))==NULL)	45	if((*pal=malloc(768))==NULL)

Lines 127-133 Link Here

(-)zgv-5.8/src/readpcx.c (-1 / +1 lines)
127	bytemax=(1<<30); /* we use a 'y<h' test instead for these files */	127	bytemax=(1<<30); /* we use a 'y<h' test instead for these files */
128		128
129	/* the normal +2 lines in case we're dithering a 24-bit file */	129	/* the normal +2 lines in case we're dithering a 24-bit file */
130	if((bmap=malloc(w(h+2)*bytepp))==NULL)	130	if(WH_BAD(w,h) \|\| (bmap=malloc(w(h+2)*bytepp))==NULL)
131	CLOSE_AND_RET(_PICERR_NOMEM);	131	CLOSE_AND_RET(_PICERR_NOMEM);
132		132
133	/* need this if more than one bitplane */	133	/* need this if more than one bitplane */

Lines 223-230 Link Here

(-)zgv-5.8/src/readpng.c (-2 / +3 lines)
223		223
224		224
225	/* allocate image memory (with two extra lines for dithering) */	225	/* allocate image memory (with two extra lines for dithering) */
226	theimage=(byte )malloc(pixelsizewidth*(height+2));	226	if(WH_BAD(width,height) \|\|
227	if(theimage==NULL) return(_PICERR_NOMEM);	227	(theimage=(byte )malloc(pixelsizewidth*(height+2)))==NULL)
		228	return(_PICERR_NOMEM);
228		229
229		230
230	ilheight=height*number_passes;	231	ilheight=height*number_passes;

Lines 144-150 Link Here

(-)zgv-5.8/src/readpnm.c (-2 / +4 lines)
144	* 3 times as much for each line, which works out only meaning	144	* 3 times as much for each line, which works out only meaning
145	* 3x as much for the last line. If you see what I mean. (!?)	145	* 3x as much for the last line. If you see what I mean. (!?)
146	*/	146	*/
147	if((bmap=malloc(w(h+2)*bytepp))==NULL)	147	if(WH_BAD(w,h) \|\| (bmap=malloc(w(h+2)*bytepp))==NULL)
148	CLOSE_AND_RET(_PICERR_NOMEM);	148	CLOSE_AND_RET(_PICERR_NOMEM);
149		149
150		150
Lines 294-299 Link Here
294		294
295	int ditherinit(int w)	295	int ditherinit(int w)
296	{	296	{
		297	if(WH_BAD(w+10,sizeof(int))) return(0);
		298
297	ditherfinish(); /* make sure any previous mem is unallocated */	299	ditherfinish(); /* make sure any previous mem is unallocated */
298	if((evenerr=calloc(3*(w+10),sizeof(int)))==NULL \|\|	300	if((evenerr=calloc(3*(w+10),sizeof(int)))==NULL \|\|
299	(odderr =calloc(3*(w+10),sizeof(int)))==NULL \|\|	301	(odderr =calloc(3*(w+10),sizeof(int)))==NULL \|\|
Lines 418-424 Link Here
418	if((maxval=read_next_number(in))!=255)	420	if((maxval=read_next_number(in))!=255)
419	return(_PICERR_CORRUPT);	421	return(_PICERR_CORRUPT);
420		422
421	if((bmap=malloc(wh))==NULL)	423	if(WH_BAD(w,h) \|\| (bmap=malloc(wh))==NULL)
422	return(_PICERR_NOMEM);	424	return(_PICERR_NOMEM);
423		425
424	count=fread(bmap,1,wh,in);	426	count=fread(bmap,1,wh,in);

Lines 184-190 Link Here

(-)zgv-5.8/src/readprf.c (-2 / +4 lines)
184	}	184	}
185		185
186	n=width*squaresize;	186	n=width*squaresize;
187	if((planebuf[0]=work_planebuf=calloc(n,planes))==NULL)	187	if(WH_BAD(width,height) \|\| (planebuf[0]=work_planebuf=calloc(n,planes))==NULL)
188	CLOSE_AND_RET(_PICERR_NOMEM);	188	CLOSE_AND_RET(_PICERR_NOMEM);
189	for(f=1;f<planes;f++)	189	for(f=1;f<planes;f++)
190	planebuf[f]=planebuf[f-1]+n;	190	planebuf[f]=planebuf[f-1]+n;
Lines 202-208 Link Here
202	}	202	}
203		203
204	/* add the usual extra 2 lines in case of dithering */	204	/* add the usual extra 2 lines in case of dithering */
205	if((bmap=work_bmap=malloc(width(height+2)*planes))==NULL)	205	/* width/height check already done, but WTF :-) */
		206	if(WH_BAD(width,height) \|\|
		207	(bmap=work_bmap=malloc(width(height+2)*planes))==NULL)
206	{	208	{
207	free(planebuf[0]);	209	free(planebuf[0]);
208	CLOSE_AND_RET(_PICERR_NOMEM);	210	CLOSE_AND_RET(_PICERR_NOMEM);

Lines 179-185 Link Here

(-)zgv-5.8/src/readtga.c (-1 / +1 lines)
179	* 3 times as much for each line, which works out only meaning	179	* 3 times as much for each line, which works out only meaning
180	* 3x as much for the last line. If you see what I mean. (!?)	180	* 3x as much for the last line. If you see what I mean. (!?)
181	*/	181	*/
182	if((bmap=malloc(w(h+2)*bytepp))==NULL)	182	if(WH_BAD(w,h) \|\| (bmap=malloc(w(h+2)*bytepp))==NULL)
183	CLOSE_AND_RET(_PICERR_NOMEM);	183	CLOSE_AND_RET(_PICERR_NOMEM);
184		184
185		185

Lines 86-92 Link Here

(-)zgv-5.8/src/readtiff.c (-1 / +2 lines)
86	* certain the dithering has room.	86	* certain the dithering has room.
87	*/	87	*/
88	numpix=width*height;	88	numpix=width*height;
89	if((image=bmap=work_bmap=malloc(numpixsizeof(uint32)+width32))==NULL)	89	if(WH_BAD(width,height) \|\|
		90	(image=bmap=work_bmap=malloc(numpixsizeof(uint32)+width32))==NULL)
90	CLOSE_AND_RET(_PICERR_NOMEM);	91	CLOSE_AND_RET(_PICERR_NOMEM);
91		92
92	/* XXX what about hffunc!? */	93	/* XXX what about hffunc!? */

Lines 97-103 Link Here

(-)zgv-5.8/src/readxbm.c (-1 / +1 lines)
97		97
98	w8=(w+7)/8;	98	w8=(w+7)/8;
99		99
100	if((bmap=image=malloc(wh))==NULL)	100	if(WH_BAD(w,h) \|\| (bmap=image=malloc(wh))==NULL)
101	CLOSE_AND_RET(_PICERR_NOMEM);	101	CLOSE_AND_RET(_PICERR_NOMEM);
102		102
103	/* save stuff in case of abort */	103	/* save stuff in case of abort */

Lines 180-186 Link Here

(-)zgv-5.8/src/readxpm.c (-2 / +2 lines)
180	if(colchars!=NULL) free(colchars);	180	if(colchars!=NULL) free(colchars);
181		181
182	/* alloc colchars array */	182	/* alloc colchars array */
183	if((colchars=malloc(ncols*sizeof(struct colchars_tag)))==NULL)	183	if(ncols>(1<<24) \|\| (colchars=malloc(ncols*sizeof(struct colchars_tag)))==NULL)
184	CLOSE_AND_RET(_PICERR_NOMEM);	184	CLOSE_AND_RET(_PICERR_NOMEM);
185		185
186		186
Lines 369-375 Link Here
369	*/	369	*/
370		370
371	/* extra lines are in case we're dithering. */	371	/* extra lines are in case we're dithering. */
372	if((bmap=malloc(w(h+2)*bytepp))==NULL)	372	if(WH_BAD(w,h) \|\| (bmap=malloc(w(h+2)*bytepp))==NULL)
373	CLOSE_AND_RET(_PICERR_NOMEM);	373	CLOSE_AND_RET(_PICERR_NOMEM);
374		374
375	ptr=*bmap;	375	ptr=*bmap;




/* make 15/16-bit colours, used in a few different places */
#define GET15BITCOLOUR(r,g,b) ((((r)&0xf8)<<7)|(((g)&0xf8)<<2)|((b)>>3))
#define GET16BITCOLOUR(r,g,b) ((((r)&0xf8)<<8)|(((g)&0xfc)<<3)|((b)>>3))

/* range check on width and height as a crude way of avoiding overflows
 * when calling malloc/calloc. The maximum we can allow is around 37000,
 * but 32767 at least makes it consistent with xzgv. :-)
 * Adds an extra 2 to height for max-height check, as we usually allocate
 * 2 more lines to allow for dithering.
 */
#define WH_MAX	32767
#define WH_BAD(w,h)	((w)<=0 || (w)>WH_MAX || (h)<=0 || ((h)+2)>WH_MAX)

Return to bug 69150

Lines 1-3 Link Here

(-)zgv-5.8/ChangeLog (+24 lines)
		1	2004-10-31 Russell Marks <russell.marks@ntlworld.com>
		2
		3	* Added width/height limits to all picture readers, 32767x32767 is
		4	now the maximum image size supported (consistent with xzgv). This
		5	is a crude (albeit effective) fix for heap overflow bugs - there
		6	may yet be more subtle problems, but I can't really fix them until
		7	I know they're there. :-) Thanks to Luke Macken for letting me
		8	know about the heap overflow problems. I suppose I should also
		9	thank "infamous41md" for publishing the original exploit (for the
		10	XPM colours bug), even if he didn't bother emailing me or
		11	anything.
		12
		13	* src/readxpm.c (read_xpm_file): fix for exploitable malloc() arg
		14	overflow. There are several more of these in zgv, but this is the
		15	easiest to fix.
		16
		17	2004-07-08 Russell Marks <russell.marks@ntlworld.com>
		18
		19	* src/readgif.c (read_gif_file): added more multiple-image (e.g.
		20	animated) GIF brokenness checks than before. Previously it was
		21	possible to get a segfault with the `right' file, despite there
		22	already being various range checks. Thanks to Mikulas Patocka for
		23	spotting this.
		24
1	2004-03-29 Russell Marks <russell.marks@ntlworld.com>	25	2004-03-29 Russell Marks <russell.marks@ntlworld.com>
2		26
3	* Version 5.8.	27	* Version 5.8.

Lines 66-68 Link Here

(-)zgv-5.8/src/zgv.h (+9 lines)
66	/* make 15/16-bit colours, used in a few different places */	66	/* make 15/16-bit colours, used in a few different places */
67	#define GET15BITCOLOUR(r,g,b) ((((r)&0xf8)<<7)\|(((g)&0xf8)<<2)\|((b)>>3))	67	#define GET15BITCOLOUR(r,g,b) ((((r)&0xf8)<<7)\|(((g)&0xf8)<<2)\|((b)>>3))
68	#define GET16BITCOLOUR(r,g,b) ((((r)&0xf8)<<8)\|(((g)&0xfc)<<3)\|((b)>>3))	68	#define GET16BITCOLOUR(r,g,b) ((((r)&0xf8)<<8)\|(((g)&0xfc)<<3)\|((b)>>3))
		69
		70	/* range check on width and height as a crude way of avoiding overflows
		71	* when calling malloc/calloc. The maximum we can allow is around 37000,
		72	* but 32767 at least makes it consistent with xzgv. :-)
		73	* Adds an extra 2 to height for max-height check, as we usually allocate
		74	* 2 more lines to allow for dithering.
		75	*/
		76	#define WH_MAX 32767
		77	#define WH_BAD(w,h) ((w)<=0 \|\| (w)>WH_MAX \|\| (h)<=0 \|\| ((h)+2)>WH_MAX)