xorl %eax, %eax

CVE-2009-3546: PHP _gdGetColors() Integer Overflow

with 2 comments

A few days ago, pajoye reported a vulnerability in GD library of PHP. The bug was finally fixed using a patch developed by Tomas Hoger of Red Hat. The vulnerable versions are limited to 5.2.11 and 5.3.0. Here is some code from ext/gd/libgd/gd_gd.c file.

/* */
/* Shared code to read color tables from gd file. */
/* */
int _gdGetColors (gdIOCtx * in, gdImagePtr im, int gd2xFlag)
{
	int i;
	if (gd2xFlag) {
		int trueColorFlag;
		if (!gdGetByte(&trueColorFlag, in)) {
			goto fail1;
		}
		/* 2.0.12: detect bad truecolor .gd files created by pre-2.0.12.
		 * Beginning in 2.0.12 truecolor is indicated by the initial 2-byte
		 * signature.
		 */
		if (trueColorFlag != im->trueColor) {
			goto fail1;
		}
		/* This should have been a word all along */
		if (!im->trueColor) {
			if (!gdGetWord(&im->colorsTotal, in)) {
				goto fail1;
			}
		}
		/* Int to accommodate truecolor single-color transparency */
		if (!gdGetInt(&im->transparent, in)) {
			goto fail1;
		}
	} else {
   ...
	}

	GD2_DBG(printf("Pallette had %d colours (T=%d)\n", im->colorsTotal, im->transparent));
   ...
	for (i = 0; i < gdMaxColors; i++) {
   ...
	}

	for (i = 0; i < im->colorsTotal; i++) {
		im->open[i] = 0;
	}

	return TRUE;
fail1:
	return FALSE;
}

In the above file, ‘im->colorsTotal’ is a user controlled variable which is used to describe the total number of colors used. From gd.h we can read…

#define gdMaxColors 256
   ...
typedef struct gdImageStruct {
   ...
	/* These are valid in palette images only. See also
		'alpha', which appears later in the structure to
		preserve binary backwards compatibility */
	int colorsTotal;
	int red[gdMaxColors];
	int green[gdMaxColors];
	int blue[gdMaxColors];
	int open[gdMaxColors];
   ...
} gdImage;

But this signed integer is never checked against the constant ‘gdMaxColors’ and allows the user to request more than 256 colors. This would result in a buffer overflow during the last ‘for’ loop of _gdGetColors() routine.
Of course, this was patched like this:

 			if (!gdGetWord(&im->colorsTotal, in)) {
 				goto fail1;
 			}
+			if (im->colorsTotal > gdMaxColors) {
+				goto fail1;
+			}
 		}
 		/* Int to accommodate truecolor single-color transparency */
 		if (!gdGetInt(&im->transparent, in)) {

Which is a simple check for ‘im->colorsTotal’ being greater than ‘gdMaxColors’ constant value.
Security researchers at vigilance.fr state that this code can be reached through calling PHP functions imagecreatefromgd() and imagecolorallocate().

Written by xorl

October 22, 2009 at 21:31

Posted in bugs

2 Responses

Subscribe to comments with RSS.

  1. wow, that bug’s been there for a very long time, kinda surprised it didn’t get fixed before.

    ilja

    November 18, 2009 at 18:02

  2. not sure if this is still accurate, but from my old notes I had 6 entrypoints for this bug:
    – gdImageCreateFromGd
    – gdImageCreateFromGdPtr
    – gdImageCreateFromGd2
    – gdImageCreateFromGd2Ptr
    – gdImageCreateFromGd2Part
    – gdImageCreateFromGd2PartPtr

    ilja

    November 18, 2009 at 18:03


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s