From 7a0fe97841dda205078ff5cd3819b4836a91b703 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Sat, 11 Jun 2016 10:12:59 -0700 Subject: [PATCH] Change image size checks This covers the case where PNG_IMAGE_BUFFER_SIZE can overflow in the application as a result of the application using an increased 'row_stride'; previously png_image_finish_read only checked for overflow on the base calculation of components. (I.e. it checked for overflow of a png_alloc_size_t number on the total number of pixel components in the output format, not the possibly padded row length and not the number of bytes, which for linear formats is twice the number of components.) --- png.h | 7 +++++++ pngread.c | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/png.h b/png.h index 0c2c6bfe3..e41a49d6d 100644 --- a/png.h +++ b/png.h @@ -3629,6 +3629,13 @@ typedef struct * png_alloc_size_t which means that the result only overflows for * ridiculously large PNG files. libpng checks and will refuse to handle * such data (the PNG is probably invalid.) + * + * Take great care over the type of 'row_stride'; libpng assumes that the + * type is png_alloc_size_t, as returned by PNG_IMAGE_ROW_STRIDE. You can + * use any type you like but libpng only checks for overflow when the type is + * png_alloc_size_t. In particular for png_uint_32 on a 64-bit system you + * must do your own overflow checking. Cast row_stride as (png_alloc_size_t) + * to avoid this (check for overflow before the cast of course!) */ #define PNG_IMAGE_SIZE(image)\ diff --git a/pngread.c b/pngread.c index ec49cdfa4..d04ff1387 100644 --- a/pngread.c +++ b/pngread.c @@ -4104,11 +4104,21 @@ png_image_finish_read(png_imagep image, png_const_colorp background, if (image->opaque != NULL && buffer != NULL && check >= png_row_stride) { - /* Now check for overflow of the image buffer calculation; check for - * (size_t) overflow here. This detects issues with the - * PNG_IMAGE_BUFFER_SIZE macro. + /* Now check for overflow of the image buffer calculation; this + * limits the whole image size to PNG_SIZE_MAX bytes. + * + * The PNG_IMAGE_BUFFER_SIZE macro is: + * + * (PNG_IMAGE_PIXEL_COMPONENT_SIZE(fmt)*height*(row_stride)) + * + * We have no way of guaranteeing that the application used the + * correct type for 'row_stride' if it used the macro, so this is + * technically not completely safe, but this is the case throughout + * libpng; the app is responsible for making sure the calcualtion of + * buffer sizes does not overflow. */ - if (image->height <= PNG_SIZE_MAX/png_row_stride) + if (image->height <= PNG_SIZE_MAX / + PNG_IMAGE_PIXEL_COMPONENT_SIZE(image->format) / check) { if ((image->format & PNG_FORMAT_FLAG_COLORMAP) == 0 || (image->colormap_entries > 0 && colormap != NULL))