After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 169113 - GIMP gifload.exe GIF file (image width)*(image height)==0 DOS vulnerability
GIMP gifload.exe GIF file (image width)*(image height)==0 DOS vulnerability
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.2.x
Other All
: Normal minor
: ---
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2005-03-03 17:17 UTC by hongzhen zhou
Modified: 2008-01-15 12:49 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description hongzhen zhou 2005-03-03 17:17:26 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:)
=================================================
Comment 1 Sven Neumann 2005-03-03 17:29:22 UTC
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.
Comment 2 Sven Neumann 2005-03-03 17:51:40 UTC
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.