From 2414bd99d8c76f92ca9272f1b1b1eff55709298a Mon Sep 17 00:00:00 2001 From: John Bowler Date: Sat, 19 Jan 2013 23:18:59 -0600 Subject: [PATCH] [libpng16] Use consistent handling of overflows in text, sPLT and unknown png_set_* APIs --- ANNOUNCE | 5 +- CHANGES | 3 +- contrib/libtests/pngunknown.c | 4 +- contrib/libtests/pngvalid.c | 2 +- png.h | 28 ++-- pngget.c | 22 +-- pngmem.c | 64 +++++++- pngpriv.h | 17 ++ pngset.c | 298 +++++++++++++++++----------------- pngstruct.h | 2 +- 10 files changed, 262 insertions(+), 183 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 891e342bb..c88d4d2c3 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -1,5 +1,5 @@ -Libpng 1.6.0beta40 - January 19, 2013 +Libpng 1.6.0beta40 - January 20, 2013 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. @@ -591,7 +591,8 @@ Version 1.6.0beta39 [January 19, 2013] Again corrected attempt at overflow detection in png_set_unknown_chunks(). Added overflow detection in png_set_sPLT() and png_set_text_2(). -Version 1.6.0beta40 [January 19, 2013] +Version 1.6.0beta40 [January 20, 2013] + Use consistent handling of overflows in text, sPLT and unknown png_set_* APIs =========================================================================== NOTICE November 17, 2012: diff --git a/CHANGES b/CHANGES index 76d383c3e..30c7b3b5f 100644 --- a/CHANGES +++ b/CHANGES @@ -4344,7 +4344,8 @@ Version 1.6.0beta39 [January 19, 2013] Again corrected attempt at overflow detection in png_set_unknown_chunks(). Added overflow detection in png_set_sPLT() and png_set_text_2(). -Version 1.6.0beta40 [January 19, 2013] +Version 1.6.0beta40 [January 20, 2013] + Use consistent handling of overflows in text, sPLT and unknown png_set_* APIs =========================================================================== NOTICE November 17, 2012: diff --git a/contrib/libtests/pngunknown.c b/contrib/libtests/pngunknown.c index b885dc89d..9d9acc0f7 100644 --- a/contrib/libtests/pngunknown.c +++ b/contrib/libtests/pngunknown.c @@ -308,7 +308,7 @@ warning(png_structp png_ptr, const char *message) } static png_uint_32 -get_valid(display *d, png_const_infop info_ptr) +get_valid(display *d, png_infop info_ptr) { png_uint_32 flags = png_get_valid(d->png_ptr, info_ptr, (png_uint_32)~0); @@ -340,7 +340,7 @@ get_valid(display *d, png_const_infop info_ptr) } static png_uint_32 -get_unknown(display *d, int def, png_const_infop info_ptr) +get_unknown(display *d, int def, png_infop info_ptr) { /* Create corresponding 'unknown' flags */ png_uint_32 flags = 0; diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index d6214518f..3ec110956 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -4754,7 +4754,7 @@ standard_check_text(png_const_structp pp, png_const_textp tp, static void standard_text_validate(standard_display *dp, png_const_structp pp, - png_const_infop pi) + png_infop pi) { png_textp tp = NULL; png_uint_32 num_text = png_get_text(pp, pi, &tp, NULL); diff --git a/png.h b/png.h index 84f6ca30b..31ac19075 100644 --- a/png.h +++ b/png.h @@ -1,7 +1,7 @@ /* png.h - header file for PNG reference library * - * libpng version 1.6.0beta40 - January 19, 2013 + * libpng version 1.6.0beta40 - January 20, 2013 * Copyright (c) 1998-2013 Glenn Randers-Pehrson * (Version 0.96 Copyright (c) 1996, 1997 Andreas Dilger) * (Version 0.88 Copyright (c) 1995, 1996 Guy Eric Schalnat, Group 42, Inc.) @@ -11,7 +11,7 @@ * Authors and maintainers: * libpng versions 0.71, May 1995, through 0.88, January 1996: Guy Schalnat * libpng versions 0.89c, June 1996, through 0.96, May 1997: Andreas Dilger - * libpng versions 0.97, January 1998, through 1.6.0beta40 - January 19, 2013: Glenn + * libpng versions 0.97, January 1998, through 1.6.0beta40 - January 20, 2013: Glenn * See also "Contributing Authors", below. * * Note about libpng version numbers: @@ -198,7 +198,7 @@ * * This code is released under the libpng license. * - * libpng versions 1.2.6, August 15, 2004, through 1.6.0beta40, January 19, 2013, are + * libpng versions 1.2.6, August 15, 2004, through 1.6.0beta40, January 20, 2013, are * Copyright (c) 2004, 2006-2013 Glenn Randers-Pehrson, and are * distributed according to the same disclaimer and license as libpng-1.2.5 * with the following individual added to the list of Contributing Authors: @@ -310,7 +310,7 @@ * Y2K compliance in libpng: * ========================= * - * January 19, 2013 + * January 20, 2013 * * Since the PNG Development group is an ad-hoc body, we can't make * an official declaration. @@ -378,7 +378,7 @@ /* Version information for png.h - this should match the version in png.c */ #define PNG_LIBPNG_VER_STRING "1.6.0beta40" #define PNG_HEADER_VERSION_STRING \ - " libpng version 1.6.0beta40 - January 19, 2013\n" + " libpng version 1.6.0beta40 - January 20, 2013\n" #define PNG_LIBPNG_VER_SONUM 16 #define PNG_LIBPNG_VER_DLLNUM 16 @@ -2182,7 +2182,7 @@ PNG_FIXED_EXPORT(140, void, png_set_gAMA_fixed, (png_const_structrp png_ptr, #ifdef PNG_hIST_SUPPORTED PNG_EXPORT(141, png_uint_32, png_get_hIST, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_uint_16p *hist)); + png_inforp info_ptr, png_uint_16p *hist)); #endif #ifdef PNG_hIST_SUPPORTED @@ -2214,7 +2214,7 @@ PNG_EXPORT(146, void, png_set_oFFs, (png_const_structrp png_ptr, #ifdef PNG_pCAL_SUPPORTED PNG_EXPORT(147, png_uint_32, png_get_pCAL, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_charp *purpose, png_int_32 *X0, + png_inforp info_ptr, png_charp *purpose, png_int_32 *X0, png_int_32 *X1, int *type, int *nparams, png_charp *units, png_charpp *params)); #endif @@ -2237,7 +2237,7 @@ PNG_EXPORT(150, void, png_set_pHYs, (png_const_structrp png_ptr, #endif PNG_EXPORT(151, png_uint_32, png_get_PLTE, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_colorp *palette, int *num_palette)); + png_inforp info_ptr, png_colorp *palette, int *num_palette)); PNG_EXPORT(152, void, png_set_PLTE, (png_structrp png_ptr, png_inforp info_ptr, png_const_colorp palette, int num_palette)); @@ -2266,7 +2266,7 @@ PNG_EXPORT(157, void, png_set_sRGB_gAMA_and_cHRM, (png_const_structrp png_ptr, #ifdef PNG_iCCP_SUPPORTED PNG_EXPORT(158, png_uint_32, png_get_iCCP, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_charpp name, int *compression_type, + png_inforp info_ptr, png_charpp name, int *compression_type, png_bytepp profile, png_uint_32 *proflen)); #endif @@ -2277,8 +2277,8 @@ PNG_EXPORT(159, void, png_set_iCCP, (png_const_structrp png_ptr, #endif #ifdef PNG_sPLT_SUPPORTED -PNG_EXPORT(160, png_uint_32, png_get_sPLT, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_sPLT_tpp entries)); +PNG_EXPORT(160, int, png_get_sPLT, (png_const_structrp png_ptr, + png_inforp info_ptr, png_sPLT_tpp entries)); #endif #ifdef PNG_sPLT_SUPPORTED @@ -2288,8 +2288,8 @@ PNG_EXPORT(161, void, png_set_sPLT, (png_const_structrp png_ptr, #ifdef PNG_TEXT_SUPPORTED /* png_get_text also returns the number of text chunks in *num_text */ -PNG_EXPORT(162, png_uint_32, png_get_text, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_textp *text_ptr, int *num_text)); +PNG_EXPORT(162, int, png_get_text, (png_const_structrp png_ptr, + png_inforp info_ptr, png_textp *text_ptr, int *num_text)); #endif /* Note while png_set_text() will accept a structure whose text, @@ -2472,7 +2472,7 @@ PNG_EXPORT(175, void, png_set_unknown_chunk_location, (png_const_structrp png_ptr, png_inforp info_ptr, int chunk, int location)); PNG_EXPORT(176, int, png_get_unknown_chunks, (png_const_structrp png_ptr, - png_const_inforp info_ptr, png_unknown_chunkpp entries)); + png_inforp info_ptr, png_unknown_chunkpp entries)); #endif /* Png_free_data() will turn off the "valid" flag for anything it frees. diff --git a/pngget.c b/pngget.c index f68027052..c2632f5b5 100644 --- a/pngget.c +++ b/pngget.c @@ -701,7 +701,7 @@ png_get_sRGB(png_const_structrp png_ptr, png_const_inforp info_ptr, #ifdef PNG_iCCP_SUPPORTED png_uint_32 PNGAPI -png_get_iCCP(png_const_structrp png_ptr, png_const_inforp info_ptr, +png_get_iCCP(png_const_structrp png_ptr, png_inforp info_ptr, png_charpp name, int *compression_type, png_bytepp profile, png_uint_32 *proflen) { @@ -726,14 +726,14 @@ png_get_iCCP(png_const_structrp png_ptr, png_const_inforp info_ptr, #endif #ifdef PNG_sPLT_SUPPORTED -png_uint_32 PNGAPI -png_get_sPLT(png_const_structrp png_ptr, png_const_inforp info_ptr, +int PNGAPI +png_get_sPLT(png_const_structrp png_ptr, png_inforp info_ptr, png_sPLT_tpp spalettes) { if (png_ptr != NULL && info_ptr != NULL && spalettes != NULL) { *spalettes = info_ptr->splt_palettes; - return ((png_uint_32)info_ptr->splt_palettes_num); + return info_ptr->splt_palettes_num; } return (0); @@ -742,7 +742,7 @@ png_get_sPLT(png_const_structrp png_ptr, png_const_inforp info_ptr, #ifdef PNG_hIST_SUPPORTED png_uint_32 PNGAPI -png_get_hIST(png_const_structrp png_ptr, png_const_inforp info_ptr, +png_get_hIST(png_const_structrp png_ptr, png_inforp info_ptr, png_uint_16p *hist) { png_debug1(1, "in %s retrieval function", "hIST"); @@ -818,7 +818,7 @@ png_get_oFFs(png_const_structrp png_ptr, png_const_inforp info_ptr, #ifdef PNG_pCAL_SUPPORTED png_uint_32 PNGAPI -png_get_pCAL(png_const_structrp png_ptr, png_const_inforp info_ptr, +png_get_pCAL(png_const_structrp png_ptr, png_inforp info_ptr, png_charp *purpose, png_int_32 *X0, png_int_32 *X1, int *type, int *nparams, png_charp *units, png_charpp *params) { @@ -938,7 +938,7 @@ png_get_pHYs(png_const_structrp png_ptr, png_const_inforp info_ptr, #endif /* pHYs */ png_uint_32 PNGAPI -png_get_PLTE(png_const_structrp png_ptr, png_const_inforp info_ptr, +png_get_PLTE(png_const_structrp png_ptr, png_inforp info_ptr, png_colorp *palette, int *num_palette) { png_debug1(1, "in %s retrieval function", "PLTE"); @@ -974,8 +974,8 @@ png_get_sBIT(png_const_structrp png_ptr, png_inforp info_ptr, #endif #ifdef PNG_TEXT_SUPPORTED -png_uint_32 PNGAPI -png_get_text(png_const_structrp png_ptr, png_const_inforp info_ptr, +int PNGAPI +png_get_text(png_const_structrp png_ptr, png_inforp info_ptr, png_textp *text_ptr, int *num_text) { if (png_ptr != NULL && info_ptr != NULL && info_ptr->num_text > 0) @@ -989,7 +989,7 @@ png_get_text(png_const_structrp png_ptr, png_const_inforp info_ptr, if (num_text != NULL) *num_text = info_ptr->num_text; - return ((png_uint_32)info_ptr->num_text); + return info_ptr->num_text; } if (num_text != NULL) @@ -1064,7 +1064,7 @@ png_get_tRNS(png_const_structrp png_ptr, png_inforp info_ptr, #ifdef PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED int PNGAPI -png_get_unknown_chunks(png_const_structrp png_ptr, png_const_inforp info_ptr, +png_get_unknown_chunks(png_const_structrp png_ptr, png_inforp info_ptr, png_unknown_chunkpp unknowns) { if (png_ptr != NULL && info_ptr != NULL && unknowns != NULL) diff --git a/pngmem.c b/pngmem.c index 5cc345093..b94e9bbf6 100644 --- a/pngmem.c +++ b/pngmem.c @@ -76,7 +76,7 @@ png_malloc_base,(png_const_structrp png_ptr, png_alloc_size_t size), #ifdef PNG_USER_MEM_SUPPORTED PNG_UNUSED(png_ptr) #endif - if (size > 0 && size <= ~(size_t)0 + if (size > 0 && size <= PNG_SIZE_MAX # ifdef PNG_MAX_MALLOC_64K && size <= 65536U # endif @@ -95,6 +95,68 @@ png_malloc_base,(png_const_structrp png_ptr, png_alloc_size_t size), return NULL; } +/* This is really here only to work round a spurious warning in GCC 4.6 and 4.7 + * that arises because of the checks in png_realloc_array that are repeated in + * png_malloc_array. + */ +static png_voidp +png_malloc_array_checked(png_const_structrp png_ptr, int nelements, + size_t element_size) +{ + png_alloc_size_t req = nelements; /* known to be > 0 */ + + if (req <= PNG_SIZE_MAX/element_size) + return png_malloc_base(png_ptr, req * element_size); + + /* The failure case when the request is too large */ + return NULL; +} + +PNG_FUNCTION(png_voidp /* PRIVATE */, +png_malloc_array,(png_const_structrp png_ptr, int nelements, + size_t element_size),PNG_ALLOCATED) +{ + if (nelements <= 0 || element_size == 0) + png_error(png_ptr, "internal error: array alloc"); + + return png_malloc_array_checked(png_ptr, nelements, element_size); +} + +PNG_FUNCTION(png_voidp /* PRIVATE */, +png_realloc_array,(png_const_structrp png_ptr, png_const_voidp old_array, + int old_elements, int add_elements, size_t element_size),PNG_ALLOCATED) +{ + /* These are internal errors: */ + if (add_elements <= 0 || element_size == 0 || old_elements < 0 || + (old_array == NULL && old_elements > 0)) + png_error(png_ptr, "internal error: array realloc"); + + /* Check for overflow on the elements count (so the caller does not have to + * check.) + */ + if (add_elements <= INT_MAX - old_elements) + { + png_voidp new_array = png_malloc_array_checked(png_ptr, + old_elements+add_elements, element_size); + + if (new_array != NULL) + { + /* Because png_malloc_array worked the size calculations below cannot + * overflow. + */ + if (old_elements > 0) + memcpy(new_array, old_array, element_size*(unsigned)old_elements); + + memset((char*)new_array + element_size*(unsigned)old_elements, 0, + element_size*(unsigned)add_elements); + + return new_array; + } + } + + return NULL; /* error */ +} + /* Various functions that have different error handling are derived from this. * png_malloc always exists, but if PNG_USER_MEM_SUPPORTED is defined a separate * function png_malloc_default is also provided. diff --git a/pngpriv.h b/pngpriv.h index 36c064960..224698ec2 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -757,6 +757,23 @@ PNG_INTERNAL_FUNCTION(int,png_user_version_check,(png_structrp png_ptr, PNG_INTERNAL_FUNCTION(png_voidp,png_malloc_base,(png_const_structrp png_ptr, png_alloc_size_t size),PNG_ALLOCATED); +#if defined PNG_TEXT_SUPPORTED || defined PNG_sPLT_SUPPORTED ||\ + defined PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED +/* Internal array allocator, outputs no error or warning messages on failure, + * just returns NULL. + */ +PNG_INTERNAL_FUNCTION(png_voidp,png_malloc_array,(png_const_structrp png_ptr, + int nelements, size_t element_size),PNG_ALLOCATED); + +/* The same but an existing array is extended by add_elements. This function + * also memsets the new elements to 0 and copies the old elements. The old + * array is not freed or altered. + */ +PNG_INTERNAL_FUNCTION(png_voidp,png_realloc_array,(png_const_structrp png_ptr, + png_const_voidp array, int old_elements, int add_elements, + size_t element_size),PNG_ALLOCATED); +#endif /* text, sPLT or unknown chunks */ + /* Magic to create a struct when there is no struct to call the user supplied * memory allocators. Because error handling has not been set up the memory * handlers can't safely call png_error, but this is an obscure and undocumented diff --git a/pngset.c b/pngset.c index 61232bf2e..11287f6cd 100644 --- a/pngset.c +++ b/pngset.c @@ -696,78 +696,62 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, png_debug1(1, "in %lx storage function", png_ptr == NULL ? "unexpected" : (unsigned long)png_ptr->chunk_name); - if (png_ptr == NULL || info_ptr == NULL || num_text == 0) + if (png_ptr == NULL || info_ptr == NULL || num_text <= 0 || text_ptr == NULL) return(0); /* Make sure we have enough space in the "text" array in info_struct - * to hold all of the incoming text_ptr objects. - * - * There were two overflow conditions here, one on the count and one on - * memory. On a 32-bit system the memory limit is critical, while on a - * 64-bit system the count limit is critical. This test defends against - * both. + * to hold all of the incoming text_ptr objects. This compare can't overflow + * because max_text >= num_text (anyway, subtract of two positive integers + * can't overflow in any case.) */ - if (num_text < 0 || - num_text > INT_MAX - info_ptr->num_text - 8 || - (unsigned int)/*SAFE*/(num_text +/*SAFE*/ - info_ptr->num_text + 8) >= - PNG_SIZE_MAX/(sizeof (png_text))) + if (num_text > info_ptr->max_text - info_ptr->num_text) { - png_warning(png_ptr, "too many text chunks"); - return(0); - } - - if (info_ptr->num_text + num_text > info_ptr->max_text) - { - int old_max_text = info_ptr->max_text; int old_num_text = info_ptr->num_text; + int max_text; + png_textp new_text = NULL; - if (info_ptr->text != NULL) + /* Calculate an appropriate max_text, checking for overflow. */ + max_text = old_num_text; + if (num_text <= INT_MAX - max_text) { - png_textp old_text; + max_text += num_text; - info_ptr->max_text = info_ptr->num_text + num_text + 8; - old_text = info_ptr->text; + /* Round up to a multiple of 8 */ + if (max_text < INT_MAX-8) + max_text = (max_text + 8) & ~0x7; - info_ptr->text = (png_textp)png_malloc_warn(png_ptr, - (png_size_t)(info_ptr->max_text * (sizeof (png_text)))); + else + max_text = INT_MAX; - if (info_ptr->text == NULL) - { - /* Restore to previous condition */ - info_ptr->max_text = old_max_text; - info_ptr->text = old_text; - return(1); - } - - memcpy(info_ptr->text, old_text, (png_size_t)(old_max_text * - (sizeof (png_text)))); - png_free(png_ptr, old_text); + /* Now allocate a new array and copy the old members in, this does all + * the overflow checks. + */ + new_text = png_voidcast(png_textp,png_realloc_array(png_ptr, + info_ptr->text, old_num_text, max_text-old_num_text, + sizeof *new_text)); } - else + if (new_text == NULL) { - info_ptr->max_text = num_text + 8; - info_ptr->num_text = 0; - info_ptr->text = (png_textp)png_malloc_warn(png_ptr, - (png_size_t)(info_ptr->max_text * (sizeof (png_text)))); - if (info_ptr->text == NULL) - { - /* Restore to previous condition */ - info_ptr->num_text = old_num_text; - info_ptr->max_text = old_max_text; - return(1); - } - info_ptr->free_me |= PNG_FREE_TEXT; + png_chunk_report(png_ptr, "too many text chunks", + PNG_CHUNK_WRITE_ERROR); + return 1; } - png_debug1(3, "allocated %d entries for info_ptr->text", - info_ptr->max_text); + png_free(png_ptr, info_ptr->text); + + info_ptr->text = new_text; + info_ptr->free_me |= PNG_FREE_TEXT; + info_ptr->max_text = max_text; + /* num_text is adjusted below as the entries are copied in */ + + png_debug1(3, "allocated %d entries for info_ptr->text", max_text); } + for (i = 0; i < num_text; i++) { - png_size_t text_length, key_len; - png_size_t lang_len, lang_key_len; + size_t text_length, key_len; + size_t lang_len, lang_key_len; png_textp textp = &(info_ptr->text[info_ptr->num_text]); if (text_ptr[i].key == NULL) @@ -776,7 +760,8 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, if (text_ptr[i].compression < PNG_TEXT_COMPRESSION_NONE || text_ptr[i].compression >= PNG_TEXT_COMPRESSION_LAST) { - png_warning(png_ptr, "text compression mode is out of range"); + png_chunk_report(png_ptr, "text compression mode is out of range", + PNG_CHUNK_WRITE_ERROR); continue; } @@ -807,7 +792,8 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, } # else /* PNG_iTXt_SUPPORTED */ { - png_warning(png_ptr, "iTXt chunk not supported"); + png_chunk_report(png_ptr, "iTXt chunk not supported", + PNG_CHUNK_WRITE_ERROR); continue; } # endif @@ -830,19 +816,22 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, textp->compression = text_ptr[i].compression; } - textp->key = (png_charp)png_malloc_warn(png_ptr, - (png_size_t) - (key_len + text_length + lang_len + lang_key_len + 4)); + textp->key = png_voidcast(png_charp,png_malloc_base(png_ptr, + key_len + text_length + lang_len + lang_key_len + 4)); if (textp->key == NULL) - return(1); + { + png_chunk_report(png_ptr, "text chunk: out of memory", + PNG_CHUNK_WRITE_ERROR); + return 1; + } png_debug2(2, "Allocated %lu bytes at %p in png_set_text", (unsigned long)(png_uint_32) (key_len + lang_len + lang_key_len + text_length + 4), textp->key); - memcpy(textp->key, text_ptr[i].key,(png_size_t)(key_len)); + memcpy(textp->key, text_ptr[i].key, key_len); *(textp->key + key_len) = '\0'; if (text_ptr[i].compression > 0) @@ -864,8 +853,7 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, } if (text_length) - memcpy(textp->text, text_ptr[i].text, - (png_size_t)(text_length)); + memcpy(textp->text, text_ptr[i].text, text_length); *(textp->text + text_length) = '\0'; @@ -886,6 +874,7 @@ png_set_text_2(png_const_structrp png_ptr, png_inforp info_ptr, info_ptr->num_text++; png_debug1(3, "transferred text chunk %d", info_ptr->num_text); } + return(0); } #endif @@ -989,86 +978,87 @@ png_set_sPLT(png_const_structrp png_ptr, */ { png_sPLT_tp np; - int i; - if (png_ptr == NULL || info_ptr == NULL || nentries <= 0 || - entries == NULL) + if (png_ptr == NULL || info_ptr == NULL || nentries <= 0 || entries == NULL) return; - /* See explanation of this in png_set_text_2(). */ - if (nentries < 0 || - nentries > INT_MAX-info_ptr->splt_palettes_num || - (unsigned int)(nentries + info_ptr->splt_palettes_num) >= - PNG_SIZE_MAX/(sizeof (png_sPLT_t))) - np=NULL; - - else - np = png_voidcast(png_sPLT_tp, png_malloc_warn(png_ptr, - (info_ptr->splt_palettes_num + nentries) * (sizeof (png_sPLT_t)))); + /* Use the internal realloc function, which checks for all the possible + * overflows. Notice that the parameters are (int) and (size_t) + */ + np = png_voidcast(png_sPLT_tp,png_realloc_array(png_ptr, + info_ptr->splt_palettes, info_ptr->splt_palettes_num, nentries, + sizeof *np)); if (np == NULL) { - png_warning(png_ptr, "No memory for sPLT palettes"); + /* Out of memory or too many chunks */ + png_chunk_report(png_ptr, "too many sPLT chunks", PNG_CHUNK_WRITE_ERROR); return; } - memcpy(np, info_ptr->splt_palettes, - info_ptr->splt_palettes_num * (sizeof (png_sPLT_t))); - png_free(png_ptr, info_ptr->splt_palettes); - info_ptr->splt_palettes=NULL; + info_ptr->splt_palettes = np; + info_ptr->free_me |= PNG_FREE_SPLT; - /* TODO: fix this, it apparently leaves NULL entries in the event of OOM - * below. - */ - for (i = 0; i < nentries; i++) + np += info_ptr->splt_palettes_num; + + do { - png_sPLT_tp to = np + info_ptr->splt_palettes_num + i; - png_const_sPLT_tp from = entries + i; png_size_t length; - /* In event of error below the name and entries fields must be set to - * NULL, otherwise libpng will crash later on while trying to free the - * uninitialized pointers. + /* Skip invalid input entries */ + if (entries->name == NULL || entries->entries == NULL) + { + /* png_handle_sPLT doesn't do this, so this is an app error */ + png_app_error(png_ptr, "png_set_sPLT: invalid sPLT"); + /* Just skip the invalid entry */ + continue; + } + + np->depth = entries->depth; + + /* In the even of out-of-memory just return - there's no point keeping on + * trying to add sPLT chunks. */ - memset(to, 0, (sizeof *to)); + length = strlen(entries->name) + 1; + np->name = png_voidcast(png_charp, png_malloc_base(png_ptr, length)); - if (from->name == NULL || from->entries == NULL) - continue; + if (np->name == NULL) + break; - length = strlen(from->name) + 1; - to->name = png_voidcast(png_charp, png_malloc_warn(png_ptr, length)); + memcpy(np->name, entries->name, length); - if (to->name == NULL) + /* IMPORTANT: we have memory now that won't get freed if something else + * goes wrong, this code must free it. png_malloc_array produces no + * warnings, use a png_chunk_report (below) if there is an error. + */ + np->entries = png_voidcast(png_sPLT_entryp, png_malloc_array(png_ptr, + entries->nentries, sizeof (png_sPLT_entry))); + + if (np->entries == NULL) { - png_warning(png_ptr, - "Out of memory while processing sPLT chunk"); - continue; + png_free(png_ptr, np->name); + break; } - memcpy(to->name, from->name, length); - to->entries = png_voidcast(png_sPLT_entryp, png_malloc_warn(png_ptr, - from->nentries * (sizeof (png_sPLT_entry)))); + np->nentries = entries->nentries; + /* This multiply can't overflow because png_malloc_array has already + * checked it when doing the allocation. + */ + memcpy(np->entries, entries->entries, + entries->nentries * sizeof (png_sPLT_entry)); - if (to->entries == NULL) - { - png_warning(png_ptr, "Out of memory while processing sPLT chunk"); - png_free(png_ptr, to->name); - to->name = NULL; - continue; - } - - memcpy(to->entries, from->entries, - from->nentries * (sizeof (png_sPLT_entry))); - - to->nentries = from->nentries; - to->depth = from->depth; + /* Note that 'continue' skips the advance of the out pointer and out + * count, so an invalid entry is not added. + */ + info_ptr->valid |= PNG_INFO_sPLT; + ++(info_ptr->splt_palettes_num); + ++np; } + while (++entries, --nentries); - info_ptr->splt_palettes = np; - info_ptr->splt_palettes_num += nentries; - info_ptr->valid |= PNG_INFO_sPLT; - info_ptr->free_me |= PNG_FREE_SPLT; + if (nentries > 0) + png_chunk_report(png_ptr, "sPLT out of memory", PNG_CHUNK_WRITE_ERROR); } #endif /* PNG_sPLT_SUPPORTED */ @@ -1092,6 +1082,9 @@ check_location(png_const_structrp png_ptr, int location) (PNG_HAVE_IHDR|PNG_HAVE_PLTE|PNG_AFTER_IDAT)); } + /* This need not be an internal error - if the app calls + * png_set_unknown_chunks on a read pointer it must get the location right. + */ if (location == 0) png_error(png_ptr, "invalid location in png_set_unknown_chunks"); @@ -1113,7 +1106,8 @@ png_set_unknown_chunks(png_const_structrp png_ptr, { png_unknown_chunkp np; - if (png_ptr == NULL || info_ptr == NULL || num_unknowns <= 0) + if (png_ptr == NULL || info_ptr == NULL || num_unknowns <= 0 || + unknowns == NULL) return; /* Check for the failure cases where support has been disabled at compile @@ -1139,35 +1133,22 @@ png_set_unknown_chunks(png_const_structrp png_ptr, } # endif - /* See the comments in png_set_text_2(). */ - if (num_unknowns < 0 || - num_unknowns > INT_MAX-info_ptr->unknown_chunks_num || - (unsigned int)(num_unknowns + - info_ptr->unknown_chunks_num) >= - PNG_SIZE_MAX/(sizeof (png_unknown_chunk))) - np=NULL; - - else - /* Prior to 1.6.0 this code used png_malloc_warn; however, this meant that - * unknown critical chunks could be lost with just a warning resulting in - * undefined behavior. Changing to png_malloc fixes this by producing a - * png_error. The (png_size_t) cast was also removed as it hides a - * potential overflow. - */ - np = png_voidcast(png_unknown_chunkp, png_malloc(png_ptr, - (info_ptr->unknown_chunks_num + num_unknowns) * - (sizeof (png_unknown_chunk)))); + /* Prior to 1.6.0 this code used png_malloc_warn; however, this meant that + * unknown critical chunks could be lost with just a warning resulting in + * undefined behavior. Now png_chunk_report is used to provide behavior + * appropriate to read or write. + */ + np = png_voidcast(png_unknown_chunkp, png_realloc_array(png_ptr, + info_ptr->unknown_chunks, info_ptr->unknown_chunks_num, num_unknowns, + sizeof *np)); if (np == NULL) { - png_warning(png_ptr, - "Out of memory while processing unknown chunk"); + png_chunk_report(png_ptr, "too many unknown chunks", + PNG_CHUNK_WRITE_ERROR); return; } - memcpy(np, info_ptr->unknown_chunks, - info_ptr->unknown_chunks_num * (sizeof (png_unknown_chunk))); - png_free(png_ptr, info_ptr->unknown_chunks); info_ptr->unknown_chunks = np; /* safe because it is initialized */ info_ptr->free_me |= PNG_FREE_UNKN; @@ -1177,24 +1158,41 @@ png_set_unknown_chunks(png_const_structrp png_ptr, /* Increment unknown_chunks_num each time round the loop to protect the * just-allocated chunk data. */ - for (; num_unknowns > 0; - --num_unknowns, ++np, ++unknowns, ++(info_ptr->unknown_chunks_num)) + for (; num_unknowns > 0; --num_unknowns, ++unknowns) { - memcpy(np->name, unknowns->name, (sizeof unknowns->name)); + memcpy(np->name, unknowns->name, (sizeof np->name)); np->name[(sizeof np->name)-1] = '\0'; - np->size = unknowns->size; np->location = check_location(png_ptr, unknowns->location); if (unknowns->size == 0) + { np->data = NULL; + np->size = 0; + } else { - /* png_error is safe here because the list is stored in png_ptr */ np->data = png_voidcast(png_bytep, - png_malloc(png_ptr, unknowns->size)); + png_malloc_base(png_ptr, unknowns->size)); + + if (np->data == NULL) + { + png_chunk_report(png_ptr, "unknown chunk: out of memory", + PNG_CHUNK_WRITE_ERROR); + /* But just skip storing the unknown chunk */ + continue; + } + memcpy(np->data, unknowns->data, unknowns->size); + np->size = unknowns->size; } + + /* These increments are skipped on out-of-memory for the data - the + * unknown chunk entry gets overwritten if the png_chunk_report returns. + * This is correct in the read case (the chunk is just dropped.) + */ + ++np; + ++(info_ptr->unknown_chunks_num); } } @@ -1208,7 +1206,7 @@ png_set_unknown_chunk_location(png_const_structrp png_ptr, png_inforp info_ptr, * TODO: add a png_app_warning in 1.7 */ if (png_ptr != NULL && info_ptr != NULL && chunk >= 0 && - (unsigned int)chunk < info_ptr->unknown_chunks_num) + chunk < info_ptr->unknown_chunks_num) { if ((location & (PNG_HAVE_IHDR|PNG_HAVE_PLTE|PNG_AFTER_IDAT)) == 0) { @@ -1536,7 +1534,7 @@ void PNGAPI png_set_chunk_cache_max (png_structrp png_ptr, png_uint_32 user_chunk_cache_max) { if (png_ptr) - png_ptr->user_chunk_cache_max = (int) user_chunk_cache_max; + png_ptr->user_chunk_cache_max = user_chunk_cache_max; } /* This function was added to libpng 1.4.1 */ diff --git a/pngstruct.h b/pngstruct.h index 8d65ae3c8..0c55fa5b4 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -435,7 +435,7 @@ struct png_struct_def /* Added in libpng-1.4.0: Total number of sPLT, text, and unknown * chunks that can be stored (0 means unlimited). */ - int user_chunk_cache_max; + png_uint_32 user_chunk_cache_max; /* Total memory that a zTXt, sPLT, iTXt, iCCP, or unknown chunk * can occupy when decompressed. 0 means unlimited.