diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index 50abd409b..c2f8bde39 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -4276,62 +4276,71 @@ make_error(png_store* const ps, png_byte const colour_type, #undef exception__env /* And clear these flags */ - ps->expect_error = 0; ps->expect_warning = 0; - /* Now write the whole image, just to make sure that the detected, or - * undetected, errro has not created problems inside libpng. - */ - if (png_get_rowbytes(pp, pi) != - transform_rowsize(pp, colour_type, bit_depth)) - png_error(pp, "row size incorrect"); + if (ps->expect_error) + ps->expect_error = 0; else { - int npasses = set_write_interlace_handling(pp, interlace_type); - int pass; + /* Now write the whole image, just to make sure that the detected, or + * undetected, errro has not created problems inside libpng. This + * doesn't work if there was a png_error in png_write_info because that + * can abort before PLTE was written. + */ + if (png_get_rowbytes(pp, pi) != + transform_rowsize(pp, colour_type, bit_depth)) + png_error(pp, "row size incorrect"); - if (npasses != npasses_from_interlace_type(pp, interlace_type)) - png_error(pp, "write: png_set_interlace_handling failed"); - - for (pass=0; pass 0) - interlace_row(buffer, buffer, - bit_size(pp, colour_type, bit_depth), w, pass, - 0/*data always bigendian*/); - else - continue; - } -# endif /* do_own_interlace */ + if (interlace_type == PNG_INTERLACE_ADAM7) + { + /* The row must not be written if it doesn't exist, + * notice that there are two conditions here, either the + * row isn't ever in the pass or the row would be but + * isn't wide enough to contribute any pixels. In fact + * the wPass test can be used to skip the whole y loop + * in this case. + */ + if (PNG_ROW_IN_INTERLACE_PASS(y, pass) && + PNG_PASS_COLS(w, pass) > 0) + interlace_row(buffer, buffer, + bit_size(pp, colour_type, bit_depth), w, pass, + 0/*data always bigendian*/); + else + continue; + } +# endif /* do_own_interlace */ - png_write_row(pp, buffer); + png_write_row(pp, buffer); + } } - } - } + } /* image writing */ - png_write_end(pp, pi); + png_write_end(pp, pi); + } /* The following deletes the file that was just written. */ store_write_reset(ps); diff --git a/pngread.c b/pngread.c index 79cbac1e4..f6b8ffabe 100644 --- a/pngread.c +++ b/pngread.c @@ -727,22 +727,20 @@ png_read_destroy(png_structrp png_ptr) png_free(png_ptr, png_ptr->read_buffer); png_ptr->read_buffer = NULL; - if ((png_ptr->free_me & PNG_FREE_PLTE) != 0) + if (png_ptr->palette != NULL) { png_free(png_ptr, png_ptr->palette); png_ptr->num_palette = 0; png_ptr->palette = NULL; } - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_PLTE); #ifdef PNG_READ_tRNS_SUPPORTED - if ((png_ptr->free_me & PNG_FREE_TRNS) != 0) + if (png_ptr->trans_alpha != NULL) { png_free(png_ptr, png_ptr->trans_alpha); png_ptr->num_trans = 0; png_ptr->trans_alpha = NULL; } - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_TRNS); #endif if (png_ptr->zstream.state != NULL) diff --git a/pngrtran.c b/pngrtran.c index 9c54978c0..6594a2850 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -5985,21 +5985,15 @@ setup_palette_cache(png_structp png_ptr, png_byte cache[8*256]) static void png_remove_PLTE_and_tRNS(png_structrp png_ptr) { - if ((png_ptr->free_me & PNG_FREE_PLTE) != 0) - { - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_PLTE); + if (png_ptr->palette != NULL) png_free(png_ptr, png_ptr->palette); - } png_ptr->palette = NULL; png_ptr->num_palette = 0; # ifdef PNG_READ_tRNS_SUPPORTED - if ((png_ptr->free_me & PNG_FREE_TRNS) != 0) - { - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_TRNS); + if (png_ptr->trans_alpha != NULL) png_free(png_ptr, png_ptr->trans_alpha); - } png_ptr->trans_alpha = NULL; png_ptr->num_trans = 0; @@ -6102,13 +6096,10 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, * entries were shifted or inverted. This could be fixed, but it would * complicate the libpng API to expose the information. */ - png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, - PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); - png_ptr->free_me |= PNG_FREE_PLTE; - /* Write the transformed palette: */ { - png_colorp palette = png_ptr->palette; + png_colorp palette = png_voidcast(png_colorp, png_calloc(png_ptr, + sizeof (png_color[PNG_MAX_PALETTE_LENGTH]))); png_const_bytep p; const int is_color = (cp->tend.format & PNG_FORMAT_FLAG_COLOR) != 0; unsigned int i; @@ -6118,6 +6109,10 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, png_byte trans_alpha[PNG_MAX_PALETTE_LENGTH]; # endif /* READ_tRNS */ + memset(palette, 0xFFU, sizeof (png_color[PNG_MAX_PALETTE_LENGTH])); + png_free(png_ptr, png_ptr->palette); + png_ptr->palette = palette; + for (i=0, p=cache.b8; itend.width; ++i) { if (is_color) @@ -6148,12 +6143,17 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, # ifdef PNG_READ_tRNS_SUPPORTED if (num_trans > 0) { - png_ptr->trans_alpha = png_voidcast(png_bytep, png_malloc(png_ptr, - PNG_MAX_PALETTE_LENGTH)); - png_ptr->free_me |= PNG_FREE_TRNS; - memcpy(png_ptr->trans_alpha, trans_alpha, num_trans); - memset(png_ptr->trans_alpha+num_trans, 0xFFU, - PNG_MAX_PALETTE_LENGTH-num_trans); + png_bytep tRNS = png_voidcast(png_bytep, png_malloc(png_ptr, + PNG_MAX_PALETTE_LENGTH)); + + memset(tRNS, 0xFFU, PNG_MAX_PALETTE_LENGTH); + + if (png_ptr->trans_alpha != NULL) + png_free(png_ptr, png_ptr->trans_alpha); + + png_ptr->trans_alpha = tRNS; + + memcpy(tRNS, trans_alpha, num_trans); png_ptr->num_trans = png_check_bits(png_ptr, num_trans, 9); } # endif /* READ_tRNS */ diff --git a/pngrutil.c b/pngrutil.c index 6336300de..7bb53bafa 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -849,8 +849,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) { png_color palette[PNG_MAX_PALETTE_LENGTH]; png_uint_32 length = png_ptr->chunk_length; - int max_palette_length, num, i; - png_colorp pal_ptr; + png_uint_32 max_palette_length, num, i; png_debug(1, "in png_handle_PLTE"); @@ -878,7 +877,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) } /* The cast is safe because 'length' is less than 3*PNG_MAX_PALETTE_LENGTH */ - num = (int)length / 3; + num = length / 3U; /* If the palette has 256 or fewer entries but is too large for the bit * depth, we don't issue an error, to preserve the behavior of previous @@ -886,21 +885,21 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) * here. */ if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) - max_palette_length = (1 << png_ptr->bit_depth); + max_palette_length = (1U << png_ptr->bit_depth); else max_palette_length = PNG_MAX_PALETTE_LENGTH; if (num > max_palette_length) num = max_palette_length; - for (i = 0, pal_ptr = palette; i < num; i++, pal_ptr++) + for (i = 0; i < num; ++i) { png_byte buf[3]; png_crc_read(png_ptr, buf, 3); - pal_ptr->red = buf[0]; - pal_ptr->green = buf[1]; - pal_ptr->blue = buf[2]; + palette[i].red = buf[0]; + palette[i].green = buf[1]; + palette[i].blue = buf[2]; } /* If we actually need the PLTE chunk (ie for a paletted image), we do @@ -911,7 +910,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) #ifndef PNG_READ_OPT_PLTE_SUPPORTED if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) #endif - png_crc_finish(png_ptr, (int) length - num * 3); + png_crc_finish(png_ptr, length - num * 3U); #ifndef PNG_READ_OPT_PLTE_SUPPORTED else if (png_crc_error(png_ptr)) /* Only if we have a CRC error */ @@ -942,16 +941,16 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) } #endif /* READ_OPT_PLTE */ - /* TODO: png_set_PLTE has the side effect of setting png_ptr->palette to its - * own copy of the palette. This has the side effect that when png_start_row - * is called (this happens after any call to png_read_update_info) the - * info_ptr palette gets changed. This is extremely unexpected and - * confusing. - * - * Fix this by not sharing the palette in this way. - */ png_set_PLTE(png_ptr, info_ptr, palette, num); + /* Ok, make our own copy since the set succeeded: */ + debug(png_ptr->palette == NULL); /* should only get set once */ + png_ptr->palette = png_voidcast(png_colorp, png_malloc(png_ptr, + sizeof (png_color[PNG_MAX_PALETTE_LENGTH]))); + memset(png_ptr->palette, 0xFFU, sizeof (png_color[PNG_MAX_PALETTE_LENGTH])); + memcpy(png_ptr->palette, info_ptr->palette, 3*num); + png_ptr->num_palette = info_ptr->num_palette; + /* The three chunks, bKGD, hIST and tRNS *must* appear after PLTE and before * IDAT. Prior to 1.6.0 this was not checked; instead the code merely * checked the apparent validity of a tRNS chunk inserted before PLTE on a @@ -1606,10 +1605,13 @@ png_handle_sPLT(png_structrp png_ptr, png_inforp info_ptr) static void png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) { + png_uint_32 num_trans; png_byte readbuf[PNG_MAX_PALETTE_LENGTH]; png_debug(1, "in png_handle_tRNS"); + png_ptr->num_trans = 0U; /* safety */ + if (info_ptr != NULL && (info_ptr->valid & PNG_INFO_tRNS)) { /* libpng 1.7.0: this used to be a benign error, but it doesn't look very @@ -1633,7 +1635,7 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } png_crc_read(png_ptr, buf, 2); - png_ptr->num_trans = 1; + num_trans = 1U; png_ptr->trans_color.gray = png_get_uint_16(buf); } @@ -1648,7 +1650,7 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } png_crc_read(png_ptr, buf, 6); - png_ptr->num_trans = 1; + num_trans = 1U; png_ptr->trans_color.red = png_get_uint_16(buf); png_ptr->trans_color.green = png_get_uint_16(buf + 2); png_ptr->trans_color.blue = png_get_uint_16(buf + 4); @@ -1656,21 +1658,18 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) else if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) { - png_uint_32 length; - /* png_find_chunk_op checks this: */ debug(png_ptr->mode & PNG_HAVE_PLTE); - length = png_ptr->chunk_length; + num_trans = png_ptr->chunk_length; - if (length > png_ptr->num_palette || length == 0) + if (num_trans > png_ptr->num_palette || num_trans == 0) { png_handle_bad_length(png_ptr); return; } - png_crc_read(png_ptr, readbuf, length); - png_ptr->num_trans = length & 0x1FF; + png_crc_read(png_ptr, readbuf, num_trans); } else @@ -1680,20 +1679,22 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } if (png_crc_finish(png_ptr, 0)) - { - png_ptr->num_trans = 0; return; + + /* Set it into the info_struct: */ + png_set_tRNS(png_ptr, info_ptr, readbuf, num_trans, &png_ptr->trans_color); + + /* Now make a copy of the buffer if one is required (palette images). */ + debug(png_ptr->trans_alpha == NULL); + if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) + { + png_ptr->trans_alpha = png_voidcast(png_bytep, + png_malloc(png_ptr, PNG_MAX_PALETTE_LENGTH)); + memset(png_ptr->trans_alpha, 0xFFU, PNG_MAX_PALETTE_LENGTH); + memcpy(png_ptr->trans_alpha, info_ptr->trans_alpha, num_trans); } - /* TODO: this is a horrible side effect in the palette case because the - * png_struct ends up with a pointer to the tRNS buffer owned by the - * png_info. Fix this. - */ - png_set_tRNS(png_ptr, info_ptr, readbuf, png_ptr->num_trans, - &(png_ptr->trans_color)); - - if (info_ptr != NULL) - png_ptr->trans_alpha = info_ptr->trans_alpha; + png_ptr->num_trans = png_check_bits(png_ptr, num_trans, 9); } #else # define png_handle_tRNS NULL diff --git a/pngset.c b/pngset.c index 83857f942..033ef1f56 100644 --- a/pngset.c +++ b/pngset.c @@ -533,30 +533,20 @@ png_set_PLTE(png_structrp png_ptr, png_inforp info_ptr, png_error(png_ptr, "Invalid palette"); #endif /* MNG_FEATURES */ - /* It may not actually be necessary to set png_ptr->palette here; - * we do it for backward compatibility with the way the png_handle_tRNS - * function used to do the allocation. - * - * 1.6.0: the above statement appears to be incorrect; something has to set - * the palette inside png_struct on read. - */ png_free_data(png_ptr, info_ptr, PNG_FREE_PLTE, 0); /* Changed in libpng-1.2.1 to allocate PNG_MAX_PALETTE_LENGTH instead * of num_palette entries, in case of an invalid PNG file or incorrect * call to png_set_PLTE() with too-large sample values. */ - png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, + info_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); if (num_palette > 0) - memcpy(png_ptr->palette, palette, num_palette * (sizeof (png_color))); - info_ptr->palette = png_ptr->palette; - info_ptr->num_palette = png_ptr->num_palette = png_check_bits(png_ptr, - num_palette, 9); + memcpy(info_ptr->palette, palette, num_palette * (sizeof (png_color))); + info_ptr->num_palette = png_check_bits(png_ptr, num_palette, 9); info_ptr->free_me |= PNG_FREE_PLTE; - info_ptr->valid |= PNG_INFO_PLTE; } @@ -962,11 +952,11 @@ png_set_tRNS(png_structrp png_ptr, png_inforp info_ptr, /* Expect png_set_PLTE to happen before png_set_tRNS, so num_palette will * be set, but this is not a requirement of the API. */ - if (png_ptr->num_palette) - max_num = png_ptr->num_palette; + if (info_ptr->num_palette) + max_num = info_ptr->num_palette; else - max_num = 1 << png_ptr->bit_depth; + max_num = 1 << info_ptr->bit_depth; if (num_trans > max_num) { diff --git a/pngstruct.h b/pngstruct.h index 43b39fae5..eadd5b9e2 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -370,17 +370,18 @@ struct png_struct_def * the hope is that such processors will generate code that is both smaller * and faster. */ +#ifdef PNG_READ_SUPPORTED png_colorp palette; /* palette from the input file */ +#endif /* READ */ #ifdef PNG_READ_tRNS_SUPPORTED png_bytep trans_alpha; /* alpha values for paletted files */ -#endif +#endif /* READ_tRNS */ png_uint_32 width; /* width of image in pixels */ png_uint_32 height; /* height of image in pixels */ png_uint_32 chunk_name; /* PNG_CHUNK() id of current chunk */ png_uint_32 chunk_length; /* Length (possibly remaining) in said chunk. */ png_uint_32 crc; /* current chunk CRC value */ - png_uint_32 free_me; /* items libpng is responsible for freeing */ unsigned int flags; /* flags (should be bit fields) */ unsigned int mode :6; /* where we are in the PNG file */ diff --git a/pngtrans.c b/pngtrans.c index 5ee0ab923..df1bc7bdc 100644 --- a/pngtrans.c +++ b/pngtrans.c @@ -580,12 +580,13 @@ init_transform_mech(png_structrp png_ptr, png_transform_control *tc, int start) #ifdef PNG_PALETTE_MAX_SUPPORTED static int -set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max) +set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max, + unsigned int format_max) /* Called whenever a new maximum pixel value is found */ { /* One of these must be true: */ # ifdef PNG_CHECK_FOR_INVALID_INDEX_SUPPORTED - if (max >= png_ptr->num_palette && !png_ptr->palette_index_check_issued) + if (max >= tr->args && !png_ptr->palette_index_check_issued) { # ifdef PNG_READ_SUPPORTED # ifdef PNG_WRITE_SUPPORTED @@ -604,7 +605,7 @@ set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max) png_ptr->palette_index_max = png_check_bits(png_ptr, max, 9); # endif - if (max == (1U << png_ptr->bit_depth)-1U) + if (max == format_max) { tr->fn = NULL; /* no point continuing once the max has been seen */ return 1; /* stop */ @@ -633,7 +634,7 @@ palette_max_1bpp(png_transformp *tr, png_transform_controlp tc) } /* If the code reaches this point there is a set pixel */ - (void)set_palette_max(tc->png_ptr, *tr, 1); + (void)set_palette_max(tc->png_ptr, *tr, 1U, 1U); } static void @@ -690,7 +691,7 @@ palette_max_2bpp(png_transformp *tr, png_transform_controlp tc) continue; /* new_max is greater than max: */ - if (set_palette_max(tc->png_ptr, *tr, new_max)) + if (set_palette_max(tc->png_ptr, *tr, new_max, 3U)) return; /* Record new_max: */ @@ -726,7 +727,7 @@ palette_max_4bpp(png_transformp *tr, png_transform_controlp tc) if (max > (*tr)->args) { - if (set_palette_max(tc->png_ptr, *tr, max)) + if (set_palette_max(tc->png_ptr, *tr, max, 15U)) return; (*tr)->args = max; @@ -752,7 +753,7 @@ palette_max_8bpp(png_transformp *tr, png_transform_controlp tc) if (max > (*tr)->args) { - if (set_palette_max(tc->png_ptr, *tr, max)) + if (set_palette_max(tc->png_ptr, *tr, max, 255U)) return; (*tr)->args = max; @@ -765,13 +766,19 @@ palette_max_init(png_transformp *tr, png_transform_controlp tc) # define png_ptr (tc->png_ptr) if ((tc->format & PNG_FORMAT_FLAG_COLORMAP) != 0) { - if (tc->init == PNG_TC_INIT_FINAL) switch (tc->bit_depth) + if (tc->init == PNG_TC_INIT_FINAL) { - case 1: (*tr)->fn = palette_max_1bpp; break; - case 2: (*tr)->fn = palette_max_2bpp; break; - case 4: (*tr)->fn = palette_max_4bpp; break; - case 8: (*tr)->fn = palette_max_8bpp; break; - default:impossible("palette bit depth"); + /* Record the palette depth to check here: */ + (*tr)->args = png_ptr->num_palette; + + switch (tc->bit_depth) + { + case 1: (*tr)->fn = palette_max_1bpp; break; + case 2: (*tr)->fn = palette_max_2bpp; break; + case 4: (*tr)->fn = palette_max_4bpp; break; + case 8: (*tr)->fn = palette_max_8bpp; break; + default:impossible("palette bit depth"); + } } } diff --git a/pngwutil.c b/pngwutil.c index 6d01aa1c8..d6998e32f 100644 --- a/pngwutil.c +++ b/pngwutil.c @@ -1319,14 +1319,6 @@ png_write_tRNS(png_structrp png_ptr, png_const_bytep trans_alpha, if (color_type == PNG_COLOR_TYPE_PALETTE) { affirm(num_trans > 0 && num_trans <= PNG_MAX_PALETTE_LENGTH); - if ((unsigned int)/*SAFE*/num_trans > png_ptr->num_palette) - { - /* This is an error which can only be reliably detected late. */ - png_app_error(png_ptr, - "Invalid number of transparent colors specified"); - return; - } - { # ifdef PNG_WRITE_INVERT_ALPHA_SUPPORTED union