Skip to content

Commit

Permalink
Fix integer overflows when loading images, see bnc #630756
Browse files Browse the repository at this point in the history 
* src/bmpcodec.c:
* src/jpgcodec.c:
* src/tifcodec.c:
	Ensure no integer overflow can occur when computing the
	stride or the total pixel size (in bytes) used to load
	pictures in memory. Fix bug #630756
  • Loading branch information
Sebastien Pouliot committed Aug 16, 2010
1 parent 49256a7 commit 59f34eb
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 20 deletions.
32 changes: 23 additions & 9 deletions src/bmpcodec.c
View file
Expand Up @@ -781,7 +781,6 @@ gdip_read_bmp_image (void *pointer, GpImage **image, ImageSource source)
int colours;
BOOL os2format = FALSE;
BOOL upsidedown = TRUE;
int size;
int size_read;
BYTE *data_read = NULL;
int line;
Expand All @@ -793,6 +792,7 @@ gdip_read_bmp_image (void *pointer, GpImage **image, ImageSource source)
ARGB green_mask = 0;
ARGB blue_mask = 0;
int red_shift = 0;
unsigned long long int size;

status = gdip_read_BITMAPINFOHEADER (pointer, &bmi, source, &os2format, &upsidedown);
if (status != Ok)
Expand Down Expand Up @@ -860,23 +860,30 @@ gdip_read_bmp_image (void *pointer, GpImage **image, ImageSource source)
result->active_bitmap->width = bmi.biWidth;
result->active_bitmap->height = bmi.biHeight;

/* biWidth and biHeight are LONG (32 bits signed integer) */
size = bmi.biWidth;

switch (result->active_bitmap->pixel_format) {
case PixelFormat1bppIndexed:
result->active_bitmap->stride = (result->active_bitmap->width + 7) / 8;
result->active_bitmap->stride = (size + 7) / 8;
break;
case PixelFormat4bppIndexed:
result->active_bitmap->stride = (result->active_bitmap->width + 1) / 2;
result->active_bitmap->stride = (size + 1) / 2;
break;
case PixelFormat8bppIndexed:
result->active_bitmap->stride = result->active_bitmap->width;
break;
case PixelFormat24bppRGB:
result->active_bitmap->stride = result->active_bitmap->width * 4;
result->active_bitmap->stride = size;
break;
default:
/* For other types, we assume 32 bit and translate into 32 bit from source format */
result->active_bitmap->pixel_format = PixelFormat32bppRGB;
result->active_bitmap->stride = result->active_bitmap->width * 4;
/* fall-thru */
case PixelFormat24bppRGB:
/* stride is a (signed) _int_ and once multiplied by 4 it should hold a value that can be allocated by GdipAlloc
* this effectively limits 'width' to 536870911 pixels */
size *= 4;
if (size > G_MAXINT32)
goto error;
result->active_bitmap->stride = size;
break;
}

Expand Down Expand Up @@ -922,7 +929,14 @@ gdip_read_bmp_image (void *pointer, GpImage **image, ImageSource source)
data_read = NULL;
}

pixels = GdipAlloc (result->active_bitmap->stride * result->active_bitmap->height);
size = result->active_bitmap->stride;
/* ensure total 'size' does not overflow an integer and fits inside our 2GB limit */
size *= result->active_bitmap->height;
if (size > G_MAXINT32) {
status = OutOfMemory;
goto error;
}
pixels = GdipAlloc (size);
if (pixels == NULL) {
status = OutOfMemory;
goto error;
Expand Down
25 changes: 19 additions & 6 deletions src/jpegcodec.c
View file
Expand Up @@ -282,6 +282,7 @@ gdip_load_jpeg_image_internal (struct jpeg_source_mgr *src, GpImage **image)
BYTE *lines[4] = {NULL, NULL, NULL, NULL};
GpStatus status;
int stride;
unsigned long long int size;

destbuf = NULL;
result = NULL;
Expand Down Expand Up @@ -323,20 +324,21 @@ gdip_load_jpeg_image_internal (struct jpeg_source_mgr *src, GpImage **image)

if (cinfo.num_components == 1) {
result->cairo_format = CAIRO_FORMAT_A8;
result->active_bitmap->stride = cinfo.image_width;
result->active_bitmap->pixel_format = PixelFormat8bppIndexed;
size = 1;
} else if (cinfo.num_components == 3) {
/* libjpeg gives us RGB for many formats and
* we convert to RGB format when needed. JPEG
* does not support alpha (transparency). */
result->cairo_format = CAIRO_FORMAT_ARGB32;
result->active_bitmap->stride = 4 * cinfo.image_width;
result->active_bitmap->pixel_format = PixelFormat24bppRGB;
size = 4;
} else if (cinfo.num_components == 4) {
result->cairo_format = CAIRO_FORMAT_ARGB32;
result->active_bitmap->stride = 4 * cinfo.image_width;
result->active_bitmap->pixel_format = PixelFormat32bppRGB;
}
size = 4;
} else
goto error;

switch (cinfo.jpeg_color_space) {
case JCS_GRAYSCALE:
Expand All @@ -360,7 +362,12 @@ gdip_load_jpeg_image_internal (struct jpeg_source_mgr *src, GpImage **image)
break;
}

stride = result->active_bitmap->stride;
size *= cinfo.image_width;
/* stride is a (signed) _int_ and once multiplied by 4 it should hold a value that can be allocated by GdipAlloc
* this effectively limits 'width' to 536870911 pixels */
if (size > G_MAXINT32)
goto error;
stride = result->active_bitmap->stride = size;

/* Request cairo-compat output */
/* libjpeg can do only following conversions,
Expand Down Expand Up @@ -397,7 +404,13 @@ gdip_load_jpeg_image_internal (struct jpeg_source_mgr *src, GpImage **image)

jpeg_start_decompress (&cinfo);

destbuf = GdipAlloc (stride * cinfo.output_height);
/* ensure total 'size' does not overflow an integer and fits inside our 2GB limit */
size *= cinfo.output_height;
if (size > G_MAXINT32) {
status = OutOfMemory;
goto error;
}
destbuf = GdipAlloc (size);
if (destbuf == NULL) {
status = OutOfMemory;
goto error;
Expand Down
23 changes: 18 additions & 5 deletions src/tiffcodec.c
View file
Expand Up @@ -1104,6 +1104,8 @@ gdip_load_tiff_image (TIFF *tiff, GpImage **image)
frame = gdip_frame_add(result, &gdip_image_frameDimension_page_guid);

for (page = 0; page < num_of_pages; page++) {
unsigned long long int size;

bitmap_data = gdip_frame_add_bitmapdata(frame);
if (bitmap_data == NULL) {
goto error;
Expand Down Expand Up @@ -1139,14 +1141,25 @@ gdip_load_tiff_image (TIFF *tiff, GpImage **image)
bitmap_data->image_flags |= ImageFlagsHasRealDPI;
}

bitmap_data->stride = tiff_image.width * 4;
/* width and height are uint32, but TIFF uses 32 bits offsets (so it's real size limit is 4GB),
* however libtiff uses signed int (int32 not uint32) as offsets so we limit ourselves to 2GB */
size = tiff_image.width;
/* stride is a (signed) _int_ and once multiplied by 4 it should hold a value that can be allocated by GdipAlloc
* this effectively limits 'width' to 536870911 pixels */
size *= sizeof (guint32);
if (size > G_MAXINT32)
goto error;
bitmap_data->stride = size;
bitmap_data->width = tiff_image.width;
bitmap_data->height = tiff_image.height;
bitmap_data->reserved = GBD_OWN_SCAN0;
bitmap_data->image_flags |= ImageFlagsColorSpaceRGB | ImageFlagsHasRealPixelSize | ImageFlagsReadOnly;

num_of_pixels = tiff_image.width * tiff_image.height;
pixbuf = GdipAlloc(num_of_pixels * sizeof(guint32));
/* ensure total 'size' does not overflow an integer and fits inside our 2GB limit */
size *= tiff_image.height;
if (size > G_MAXINT32)
goto error;
pixbuf = GdipAlloc (size);
if (pixbuf == NULL) {
goto error;
}
Expand All @@ -1168,9 +1181,9 @@ gdip_load_tiff_image (TIFF *tiff, GpImage **image)
memcpy(pixbuf + (bitmap_data->stride * (tiff_image.height - i - 1)), pixbuf_row, bitmap_data->stride);
}

/* Now flip from ARGB to ABGR */
/* Now flip from ARGB to ABGR processing one pixel (4 bytes) at the time */
pixbuf_ptr = (guint32 *)pixbuf;
for (i = 0; i < num_of_pixels; i++) {
for (i = 0; i < (size >> 2); i++) {
*pixbuf_ptr = (*pixbuf_ptr & 0xff000000) |
((*pixbuf_ptr & 0x00ff0000) >> 16) |
(*pixbuf_ptr & 0x0000ff00) |
Expand Down

0 comments on commit 59f34eb

Please sign in to comment.