GNOME Bugzilla – Bug 169113
GIMP gifload.exe GIF file (image width)*(image height)==0 DOS vulnerability
Last modified: 2008-01-15 12:49:16 UTC
Distribution/Version: windows xp Change a normal gif file's image width value or image height value to 0, open this gif file using GIMP.exe, then the gifload.exe crashed! I tested it using GIMP 2.2.3 & 2.0.5 for windows. ============================================== code from gimp-2.2.4\plug-ins\common\gifload.c ============================================== ---------------------------------------------- // load_image() ---------------------------------------------- // Didn't check the value of width and height fields in image descriptor when read --------------- if (!useGlobalColormap) { if (ReadColorMap (fd, bitPixel, localColorMap, &grayScale)) { g_message ("Error reading local colormap"); return image_ID; /* will be -1 if failed on first image! */ } image_ID = ReadImage (fd, filename, LM_to_uint (buf[4], buf[5]), LM_to_uint (buf[6], buf[7]), localColorMap, bitPixel, grayScale, BitSet (buf[8], INTERLACE), imageCount, (guint) LM_to_uint (buf[0], buf[1]), (guint) LM_to_uint (buf[2], buf[3]), GifScreen.Width, GifScreen.Height ); } else { image_ID = ReadImage (fd, filename, LM_to_uint (buf[4], buf[5]), LM_to_uint (buf[6], buf[7]), GifScreen.ColorMap, GifScreen.BitPixel, GifScreen.GrayScale, BitSet (buf[8], INTERLACE), imageCount, (guint) LM_to_uint (buf[0], buf[1]), (guint) LM_to_uint (buf[2], buf[3]), GifScreen.Width, GifScreen.Height ); } ----------------------------------------------- // ReadImage() ----------------------------------------------- // Didn't check the the size when passed it to g_malloc() and didn't check the return value of g_malloc() --------------- if (alpha_frame) dest = (guchar *) g_malloc (len * height * (promote_to_rgb ? 4 : 2)); else dest = (guchar *) g_malloc (len * height); --------------- if (promote_to_rgb) { temp = dest + ( (ypos * len) + xpos ) * 4; // temp = 0 + ( (0 * 0) + 0) * 4; // temp == 0 // So it cause write access exception! *(temp ) = (guchar) cmap[0][v]; *(temp+1) = (guchar) cmap[1][v]; *(temp+2) = (guchar) cmap[2][v]; *(temp+3) = (guchar) ((v == Gif89.transparent) ? 0 : 255); } else { temp = dest + ( (ypos * len) + xpos ) * 2; // temp = 0 + ( (0 * 0) + 0) * 2; *temp = (guchar) v; *(temp+1) = (guchar) ((v == Gif89.transparent) ? 0 : 255); } ------------------------------------------------- That's all:) =================================================
This is a very minor problem only. The only thing that may crash here is plug-in. Also your code analysis is not correct. 0 is a perfectly valid argument to pass to g_malloc() and the return value of g_malloc() doesn't need to be checked.
Added a sanity check in CVS HEAD. I don't consider this worth to be backported to 2.2, closing as FIXED. 2005-03-03 Sven Neumann <sven@gimp.org> * plug-ins/common/gifload.c (ReadImage): added a sanity check for bogus frame dimensions. Fixes bug #169113.