Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix integer overflows when loading images, see bnc #630756

* 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...
commit 9b37ccf96050d029cc82471b8f4168f1496f8877 1 parent 53dbe12
Sebastien Pouliot authored
Showing with 60 additions and 20 deletions.
  1. +23 −9 src/bmpcodec.c
  2. +19 −6 src/jpegcodec.c
  3. +18 −5 src/tiffcodec.c
View
32 src/bmpcodec.c
@@ -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;
@@ -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)
@@ -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;
}
@@ -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;
View
25 src/jpegcodec.c
@@ -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;
@@ -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:
@@ -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,
@@ -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;
View
23 src/tiffcodec.c
@@ -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;
@@ -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;
}
@@ -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) |
Please sign in to comment.
Something went wrong with that request. Please try again.