From bc86b7a8a15a16438b9244cc26cc47682b9fcbb0 Mon Sep 17 00:00:00 2001 From: James Zern Date: Fri, 7 Oct 2016 13:18:29 -0700 Subject: [PATCH] imageio_util: add ImgIoUtilCheckSizeArgumentsOverflow and use it to validate decoder allocations. fixes a crash in jpegdec at least. BUG=webp:312 Change-Id: Ia940590098f29510add6aad10a8dfe9e9ea46bf4 --- imageio/imageio_util.c | 7 +++++++ imageio/imageio_util.h | 5 +++++ imageio/jpegdec.c | 15 +++++++++++---- imageio/pngdec.c | 13 ++++++++++--- imageio/tiffdec.c | 13 ++++++++++++- imageio/wicdec.c | 15 ++++++++++----- 6 files changed, 55 insertions(+), 13 deletions(-) diff --git a/imageio/imageio_util.c b/imageio/imageio_util.c index b3ec287f..eabab9dd 100644 --- a/imageio/imageio_util.c +++ b/imageio/imageio_util.c @@ -134,3 +134,10 @@ void ImgIoUtilCopyPlane(const uint8_t* src, int src_stride, } // ----------------------------------------------------------------------------- + +int ImgIoUtilCheckSizeArgumentsOverflow(uint64_t nmemb, size_t size) { + const uint64_t total_size = nmemb * size; + return (total_size == (size_t)total_size); +} + +// ----------------------------------------------------------------------------- diff --git a/imageio/imageio_util.h b/imageio/imageio_util.h index f97de69b..b44f59fd 100644 --- a/imageio/imageio_util.h +++ b/imageio/imageio_util.h @@ -49,6 +49,11 @@ int ImgIoUtilWriteFile(const char* const file_name, void ImgIoUtilCopyPlane(const uint8_t* src, int src_stride, uint8_t* dst, int dst_stride, int width, int height); +//------------------------------------------------------------------------------ + +// Returns 0 in case of overflow of nmemb * size. +int ImgIoUtilCheckSizeArgumentsOverflow(uint64_t nmemb, size_t size); + #ifdef __cplusplus } // extern "C" #endif diff --git a/imageio/jpegdec.c b/imageio/jpegdec.c index 12cc7f2d..315fb27c 100644 --- a/imageio/jpegdec.c +++ b/imageio/jpegdec.c @@ -25,6 +25,7 @@ #include #include "webp/encode.h" +#include "./imageio_util.h" #include "./metadata.h" // ----------------------------------------------------------------------------- @@ -257,7 +258,8 @@ int ReadJPEG(const uint8_t* const data, size_t data_size, WebPPicture* const pic, int keep_alpha, Metadata* const metadata) { volatile int ok = 0; - int stride, width, height; + int width, height; + int64_t stride; volatile struct jpeg_decompress_struct dinfo; struct my_error_mgr jerr; uint8_t* volatile rgb = NULL; @@ -296,9 +298,14 @@ int ReadJPEG(const uint8_t* const data, size_t data_size, width = dinfo.output_width; height = dinfo.output_height; - stride = dinfo.output_width * dinfo.output_components * sizeof(*rgb); + stride = (int64_t)dinfo.output_width * dinfo.output_components * sizeof(*rgb); - rgb = (uint8_t*)malloc(stride * height); + if (stride != (int)stride || + !ImgIoUtilCheckSizeArgumentsOverflow(stride, height)) { + goto End; + } + + rgb = (uint8_t*)malloc((size_t)stride * height); if (rgb == NULL) { goto End; } @@ -325,7 +332,7 @@ int ReadJPEG(const uint8_t* const data, size_t data_size, // WebP conversion. pic->width = width; pic->height = height; - ok = WebPPictureImportRGB(pic, rgb, stride); + ok = WebPPictureImportRGB(pic, rgb, (int)stride); if (!ok) goto Error; End: diff --git a/imageio/pngdec.c b/imageio/pngdec.c index 48eb457e..480210d1 100644 --- a/imageio/pngdec.c +++ b/imageio/pngdec.c @@ -24,6 +24,7 @@ #include #include "webp/encode.h" +#include "./imageio_util.h" #include "./metadata.h" static void PNGAPI error_function(png_structp png, png_const_charp error) { @@ -216,7 +217,7 @@ int ReadPNG(const uint8_t* const data, size_t data_size, int p; volatile int ok = 0; png_uint_32 width, height, y; - png_uint_32 stride; + int64_t stride; uint8_t* volatile rgb = NULL; context.data = data; @@ -269,8 +270,14 @@ int ReadPNG(const uint8_t* const data, size_t data_size, num_passes = png_set_interlace_handling(png); png_read_update_info(png, info); - stride = (has_alpha ? 4 : 3) * width * sizeof(*rgb); - rgb = (uint8_t*)malloc(stride * height); + + stride = (int64_t)(has_alpha ? 4 : 3) * width * sizeof(*rgb); + if (stride != (int)stride || + !ImgIoUtilCheckSizeArgumentsOverflow(stride, height)) { + goto Error; + } + + rgb = (uint8_t*)malloc((size_t)stride * height); if (rgb == NULL) goto Error; for (p = 0; p < num_passes; ++p) { for (y = 0; y < height; ++y) { diff --git a/imageio/tiffdec.c b/imageio/tiffdec.c index dc980dd4..bfcccf85 100644 --- a/imageio/tiffdec.c +++ b/imageio/tiffdec.c @@ -22,6 +22,7 @@ #include #include "webp/encode.h" +#include "./imageio_util.h" #include "./metadata.h" static const struct { @@ -124,6 +125,7 @@ int ReadTIFF(const uint8_t* const data, size_t data_size, MySize, MyMapFile, MyUnmapFile); uint32 width, height; uint32* raster; + int64_t alloc_size; int ok = 0; tdir_t dircount; @@ -144,7 +146,16 @@ int ReadTIFF(const uint8_t* const data, size_t data_size, fprintf(stderr, "Error! Cannot retrieve TIFF image dimensions.\n"); goto End; } - raster = (uint32*)_TIFFmalloc(width * height * sizeof(*raster)); + + if (!ImgIoUtilCheckSizeArgumentsOverflow((uint64_t)width * height, + sizeof(*raster))) { + goto End; + } + // _Tiffmalloc uses a signed type for size. + alloc_size = (int64_t)((uint64_t)width * height * sizeof(*raster)); + if (alloc_size < 0 || alloc_size != (tmsize_t)alloc_size) goto End; + + raster = (uint32*)_TIFFmalloc((tmsize_t)alloc_size); if (raster != NULL) { if (TIFFReadRGBAImageOriented(tif, width, height, raster, ORIENTATION_TOPLEFT, 1)) { diff --git a/imageio/wicdec.c b/imageio/wicdec.c index b7ed7c3c..d0392494 100644 --- a/imageio/wicdec.c +++ b/imageio/wicdec.c @@ -274,7 +274,7 @@ int ReadPictureWithWIC(const char* const filename, NULL }; int has_alpha = 0; - int stride; + int64_t stride; IFS(CoInitialize(NULL)); IFS(CoCreateInstance(MAKE_REFGUID(CLSID_WICImagingFactory), NULL, @@ -334,14 +334,19 @@ int ReadPictureWithWIC(const char* const filename, // Decode. IFS(IWICFormatConverter_GetSize(converter, &width, &height)); - stride = importer->bytes_per_pixel * width * sizeof(*rgb); + stride = (int64_t)importer->bytes_per_pixel * width * sizeof(*rgb); + if (stride != (int)stride || + !ImgIoUtilCheckSizeArgumentsOverflow(stride, height)) { + hr = E_FAIL; + } + if (SUCCEEDED(hr)) { - rgb = (BYTE*)malloc(stride * height); + rgb = (BYTE*)malloc((size_t)stride * height); if (rgb == NULL) hr = E_OUTOFMEMORY; } IFS(IWICFormatConverter_CopyPixels(converter, NULL, - stride, stride * height, rgb)); + (UINT)stride, (UINT)stride * height, rgb)); // WebP conversion. if (SUCCEEDED(hr)) { @@ -349,7 +354,7 @@ int ReadPictureWithWIC(const char* const filename, pic->width = width; pic->height = height; pic->use_argb = 1; // For WIC, we always force to argb - ok = importer->import(pic, rgb, stride); + ok = importer->import(pic, rgb, (int)stride); if (!ok) hr = E_FAIL; } if (SUCCEEDED(hr)) {