[fwd] Debian Bug report logs - #414045 - BREAKS XLIB

[Julien Cristau]

Hi,

I received the following report, claiming that the fix for CVE-2007-1667
is wrong.  Can someone who has some clue about that code check that?

Thanks,
Julien

----- Forwarded message from Jan Rubaek-Pedersen <jrp at ccieurope.com> -----

Hi Julien

Sorry if this is not the right way of contacting you - I'm not sure
where to go with this one.

The "fix" made to XCreateImage breaks the protocol between client and
Xlib because the extended test makes a wrong assumption.

XCreateImage now compares the servers bits-per-pixel with the
bits_per_pixel in the supplied image (for ZPixmap). 

In our case we have code using 24 bits_per_pixel for depth 24 images (3
bytes per pixel). Many X servers use 32 bits_per_pixel for depth 24
pixmaps.

The "fix" now require that image data MUST use the same bits_per_pixel
as the X-server!

We have had a lot of problems as the "fix" is being distributed as part
of Sun Solaris security updates!


The code in XCreateImage:

XImage *XCreateImage (
    register Display *dpy,
    register Visual *visual,
    unsigned int depth,
    int format,
    int offset, /*How many pixels from the start of the data does the
		picture to be transmitted start?*/

    char *data,
    unsigned int width,
    unsigned int height,
    int xpad,
    int image_bytes_per_line) 
		/*How many bytes between a pixel on one line and the pixel with
		  the same X coordinate on the next line? 0 means
		  XCreateImage can calculate it.*/
{
	register XImage *image;
	int bits_per_pixel = 1;
	int min_bytes_per_line;

	if (depth == 0 || depth > 32 ||
	    (format != XYBitmap && format != XYPixmap && format != ZPixmap) ||
	    (format == XYBitmap && depth != 1) ||
	    (xpad != 8 && xpad != 16 && xpad != 32) ||
	    offset < 0)
	    return (XImage *) NULL;
	if ((image = (XImage *) Xcalloc(1, (unsigned) sizeof(XImage))) == NULL)
	    return (XImage *) NULL;

	image->width = width;
	image->height = height;
	image->format = format;
	image->byte_order = dpy->byte_order;
	image->bitmap_unit = dpy->bitmap_unit;
	image->bitmap_bit_order = dpy->bitmap_bit_order;
	if (visual != NULL) {
		image->red_mask = visual->red_mask;
		image->green_mask = visual->green_mask;
		image->blue_mask = visual->blue_mask;
	}
	else {
		image->red_mask = image->green_mask = image->blue_mask = 0;
	}
	if (format == ZPixmap) 
	{
	    bits_per_pixel = _XGetBitsPerPixel(dpy, (int) depth);                           ######## GETS SERVER BITS PER PIXEL IN PIXMAPS
	}

	image->xoffset = offset;
	image->bitmap_pad = xpad;
	image->depth = depth;
 	image->data = data;
	/*
	 * compute per line accelerator.
	 */
	{
	if (format == ZPixmap)
	    min_bytes_per_line = 
	       ROUNDUP((bits_per_pixel * width), image->bitmap_pad);
	else
	    min_bytes_per_line =
	        ROUNDUP((width + offset), image->bitmap_pad);
	}
	if (image_bytes_per_line == 0) {
	    image->bytes_per_line = min_bytes_per_line;
	} else if (image_bytes_per_line < min_bytes_per_line) {                             ########## WRONG TEST!
	    return 0;                                                                       ########## MEMORY LEAK!
	} else {
	    image->bytes_per_line = image_bytes_per_line;
	}

	image->bits_per_pixel = bits_per_pixel;
	image->obdata = NULL;
	_XInitImageFuncPtrs (image);

	return image;
}


/jan
----- End forwarded message -----