From 2dca15686fadb1b8951cb29b02bad4cae73448da Mon Sep 17 00:00:00 2001 From: Glenn Randers-Pehrson Date: Fri, 4 Aug 2017 14:09:27 -0500 Subject: [PATCH] [libpng16] Moved chunk-length check into a png_check_chunk_length() private function (Suggested by Max Stepin). --- ANNOUNCE | 6 +++-- CHANGES | 4 +++- pngpread.c | 34 ++------------------------ pngpriv.h | 3 +++ pngrutil.c | 70 ++++++++++++++++++++++++++++++------------------------ 5 files changed, 51 insertions(+), 66 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 24763fd8b..45f6fcf63 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -1,4 +1,4 @@ -Libpng 1.6.32beta10 - August 3, 2017 +Libpng 1.6.32beta10 - August 4, 2017 This is not intended to be a public release. It will be replaced within a few weeks by a public version or by another test version. @@ -69,7 +69,9 @@ Version 1.6.32beta09 [August 3, 2017] no longer using deprecated cmake LOCATION feature (Clifford Yapp). Fixed five-byte error in the calculation of IDAT maximum possible size. -Version 1.6.32beta10 [August 3, 2017] +Version 1.6.32beta10 [August 4, 2017] + Moved chunk-length check into a png_check_chunk_length() private + function (Suggested by Max Stepin). Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/CHANGES b/CHANGES index cfa4e7590..30bc6c989 100644 --- a/CHANGES +++ b/CHANGES @@ -5952,7 +5952,9 @@ Version 1.6.32beta09 [August 3, 2017] no longer using deprecated cmake LOCATION feature (Clifford Yapp). Fixed five-byte error in the calculation of IDAT maximum possible size. -Version 1.6.32beta10 [August 3, 2017] +Version 1.6.32beta10 [August 4, 2017] + Moved chunk-length check into a png_check_chunk_length() private + function (Suggested by Max Stepin). Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/pngpread.c b/pngpread.c index c7721461a..b1f242c9c 100644 --- a/pngpread.c +++ b/pngpread.c @@ -190,6 +190,8 @@ png_push_read_chunk(png_structrp png_ptr, png_inforp info_ptr) png_crc_read(png_ptr, chunk_tag, 4); png_ptr->chunk_name = PNG_CHUNK_FROM_STRING(chunk_tag); png_check_chunk_name(png_ptr, png_ptr->chunk_name); + png_check_chunk_length(png_ptr, png_ptr->chunk_name, + png_ptr->push_length); png_ptr->mode |= PNG_HAVE_CHUNK_HEADER; } @@ -224,38 +226,6 @@ png_push_read_chunk(png_structrp png_ptr, png_inforp info_ptr) png_benign_error(png_ptr, "Too many IDATs found"); } - if (chunk_name == png_IDAT) - { - size_t row_factor = - (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1) - + 1 + (png_ptr->interlaced? 6: 0)); - if (png_ptr->height > PNG_UINT_32_MAX/row_factor) - limit=PNG_UINT_31_MAX; - else - limit = png_ptr->height * row_factor; - limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */ - limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX; - } - else - { -# ifdef PNG_SET_USER_LIMITS_SUPPORTED - if (png_ptr->user_chunk_malloc_max > 0 && - png_ptr->user_chunk_malloc_max < limit) - limit = png_ptr->user_chunk_malloc_max; -# elif PNG_USER_CHUNK_MALLOC_MAX > 0 - if (PNG_USER_CHUNK_MALLOC_MAX < limit) - limit = PNG_USER_CHUNK_MALLOC_MAX; -# endif - } - if (png_ptr->push_length > limit) - { - printf(" png_ptr->push_length = %lu, limit = %lu\n", - (unsigned long)png_ptr->push_length,(unsigned long)limit); - png_debug2(0," png_ptr->push_length = %lu, limit = %lu", - (unsigned long)png_ptr->push_length,(unsigned long)limit); - png_chunk_error(png_ptr, "chunk data is too large"); - } - if (chunk_name == png_IHDR) { if (png_ptr->push_length != 13) diff --git a/pngpriv.h b/pngpriv.h index 823919b4c..fc222bde1 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -1530,6 +1530,9 @@ PNG_INTERNAL_FUNCTION(void,png_handle_zTXt,(png_structrp png_ptr, PNG_INTERNAL_FUNCTION(void,png_check_chunk_name,(png_structrp png_ptr, png_uint_32 chunk_name),PNG_EMPTY); +PNG_INTERNAL_FUNCTION(void,png_check_chunk_length,(png_structrp png_ptr, + png_uint_32 chunk_name, png_uint_32 chunk_length),PNG_EMPTY); + PNG_INTERNAL_FUNCTION(void,png_handle_unknown,(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length, int keep),PNG_EMPTY); /* This is the function that gets called for unknown chunks. The 'keep' diff --git a/pngrutil.c b/pngrutil.c index 476837700..b744a51d9 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -157,7 +157,6 @@ png_read_chunk_header(png_structrp png_ptr) { png_byte buf[8]; png_uint_32 length; - png_alloc_size_t limit = PNG_UINT_31_MAX; #ifdef PNG_IO_STATE_SUPPORTED png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_HDR; @@ -183,36 +182,7 @@ png_read_chunk_header(png_structrp png_ptr) png_check_chunk_name(png_ptr, png_ptr->chunk_name); /* Check for too-large chunk length */ - if (png_ptr->chunk_name != png_IDAT) - { -# ifdef PNG_SET_USER_LIMITS_SUPPORTED - if (png_ptr->user_chunk_malloc_max > 0 && - png_ptr->user_chunk_malloc_max < limit) - limit = png_ptr->user_chunk_malloc_max; -# elif PNG_USER_CHUNK_MALLOC_MAX > 0 - if (PNG_USER_CHUNK_MALLOC_MAX < limit) - limit = PNG_USER_CHUNK_MALLOC_MAX; -# endif - } - else - { - size_t row_factor = - (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1) - + 1 + (png_ptr->interlaced? 6: 0)); - if (png_ptr->height > PNG_UINT_32_MAX/row_factor) - limit=PNG_UINT_31_MAX; - else - limit = png_ptr->height * row_factor; - limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */ - limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX; - } - - if (length > limit) - { - png_debug2(0," length = %lu, limit = %lu", - (unsigned long)length,(unsigned long)limit); - png_chunk_error(png_ptr, "chunk data is too large"); - } + png_check_chunk_length(png_ptr, png_ptr->chunk_name, length); #ifdef PNG_IO_STATE_SUPPORTED png_ptr->io_state = PNG_IO_READING | PNG_IO_CHUNK_DATA; @@ -3134,6 +3104,44 @@ png_check_chunk_name(png_structrp png_ptr, png_uint_32 chunk_name) } } +void /* PRIVATE */ +png_check_chunk_length(png_structrp png_ptr, png_uint_32 chunk_name, + png_uint_32 length) +{ + png_alloc_size_t limit = PNG_UINT_31_MAX; + + if (png_ptr->chunk_name != png_IDAT) + { +# ifdef PNG_SET_USER_LIMITS_SUPPORTED + if (png_ptr->user_chunk_malloc_max > 0 && + png_ptr->user_chunk_malloc_max < limit) + limit = png_ptr->user_chunk_malloc_max; +# elif PNG_USER_CHUNK_MALLOC_MAX > 0 + if (PNG_USER_CHUNK_MALLOC_MAX < limit) + limit = PNG_USER_CHUNK_MALLOC_MAX; +# endif + } + else + { + size_t row_factor = + (png_ptr->width * png_ptr->channels * (png_ptr->bit_depth > 8? 2: 1) + + 1 + (png_ptr->interlaced? 6: 0)); + if (png_ptr->height > PNG_UINT_32_MAX/row_factor) + limit=PNG_UINT_31_MAX; + else + limit = png_ptr->height * row_factor; + limit += 6 + 5*(limit/32566+1); /* zlib+deflate overhead */ + limit=limit < PNG_UINT_31_MAX? limit : PNG_UINT_31_MAX; + } + + if (length > limit) + { + png_debug2(0," length = %lu, limit = %lu", + (unsigned long)length,(unsigned long)limit); + png_chunk_error(png_ptr, "chunk data is too large"); + } +} + /* Combines the row recently read in with the existing pixels in the row. This * routine takes care of alpha and transparency if requested. This routine also * handles the two methods of progressive display of interlaced images,