From: Matthieu Herrb Subject: Re: [update] png to 1.6.57 To: Theo de Raadt Cc: Theo Buehler , Matthieu Herrb , ports@openbsd.org Date: Thu, 9 Apr 2026 07:37:41 +0200 On Wed, Apr 08, 2026 at 11:29:17PM -0600, Theo de Raadt wrote: > Theo Buehler wrote: > > > On Thu, Apr 09, 2026 at 07:20:33AM +0200, Matthieu Herrb wrote: > > > === CVE-2026-34757 === > > > > > > Use-after-free in png_set_PLTE, png_set_tRNS and png_set_hIST > > > leading to corrupted chunk data and potential heap information > > > disclosure > > > > > > no API/ABI change. > > > > > > ok ? > > How did you see that in the mail? > > > > I'll also take case of updating the embedded copy in xenocara, > > > used by freetype, although the affected functions are not called by > > > freetype afaict. > > > > The diff between the two version reads fine and completely risk-free to > > me. ok for this as well. Thanks! > > As I told matthieu, I think we ran into a case where ports and xenocara > API/ABI needed to very sync'd before, so we need to make sure that both > naddy and I agree on the ABI/API now that we are locked. > > I think I agree, as long as I know no ABI change. This is the actual code diff for libpng.so. There is a lot more churn due to version change (and an added test), the full diff is in ~matthieu/libpng-1.6.57.diff on cvs. Index: pngrtran.c =================================================================== RCS file: /cvs/OpenBSD/xenocara/lib/libpng/pngrtran.c,v diff -u -p -u -r1.6 pngrtran.c --- pngrtran.c 26 Mar 2026 19:57:02 -0000 1.6 +++ pngrtran.c 9 Apr 2026 05:30:08 -0000 @@ -2379,7 +2379,7 @@ png_do_unpack(png_row_infop row_info, pn } row_info->bit_depth = 8; row_info->pixel_depth = (png_byte)(8 * row_info->channels); - row_info->rowbytes = row_width * row_info->channels; + row_info->rowbytes = (size_t)row_width * row_info->channels; } } #endif @@ -2581,7 +2581,7 @@ png_do_scale_16_to_8(png_row_infop row_i row_info->bit_depth = 8; row_info->pixel_depth = (png_byte)(8 * row_info->channels); - row_info->rowbytes = row_info->width * row_info->channels; + row_info->rowbytes = (size_t)row_info->width * row_info->channels; } } #endif @@ -2609,7 +2609,7 @@ png_do_chop(png_row_infop row_info, png_ row_info->bit_depth = 8; row_info->pixel_depth = (png_byte)(8 * row_info->channels); - row_info->rowbytes = row_info->width * row_info->channels; + row_info->rowbytes = (size_t)row_info->width * row_info->channels; } } #endif @@ -2845,7 +2845,7 @@ png_do_read_filler(png_row_infop row_inf *(--dp) = lo_filler; row_info->channels = 2; row_info->pixel_depth = 16; - row_info->rowbytes = row_width * 2; + row_info->rowbytes = (size_t)row_width * 2; } else @@ -2860,7 +2860,7 @@ png_do_read_filler(png_row_infop row_inf } row_info->channels = 2; row_info->pixel_depth = 16; - row_info->rowbytes = row_width * 2; + row_info->rowbytes = (size_t)row_width * 2; } } @@ -2883,7 +2883,7 @@ png_do_read_filler(png_row_infop row_inf *(--dp) = hi_filler; row_info->channels = 2; row_info->pixel_depth = 32; - row_info->rowbytes = row_width * 4; + row_info->rowbytes = (size_t)row_width * 4; } else @@ -2900,7 +2900,7 @@ png_do_read_filler(png_row_infop row_inf } row_info->channels = 2; row_info->pixel_depth = 32; - row_info->rowbytes = row_width * 4; + row_info->rowbytes = (size_t)row_width * 4; } } #endif @@ -2924,7 +2924,7 @@ png_do_read_filler(png_row_infop row_inf *(--dp) = lo_filler; row_info->channels = 4; row_info->pixel_depth = 32; - row_info->rowbytes = row_width * 4; + row_info->rowbytes = (size_t)row_width * 4; } else @@ -2941,7 +2941,7 @@ png_do_read_filler(png_row_infop row_inf } row_info->channels = 4; row_info->pixel_depth = 32; - row_info->rowbytes = row_width * 4; + row_info->rowbytes = (size_t)row_width * 4; } } @@ -2968,7 +2968,7 @@ png_do_read_filler(png_row_infop row_inf *(--dp) = hi_filler; row_info->channels = 4; row_info->pixel_depth = 64; - row_info->rowbytes = row_width * 8; + row_info->rowbytes = (size_t)row_width * 8; } else @@ -2990,7 +2990,7 @@ png_do_read_filler(png_row_infop row_inf row_info->channels = 4; row_info->pixel_depth = 64; - row_info->rowbytes = row_width * 8; + row_info->rowbytes = (size_t)row_width * 8; } } #endif @@ -4484,7 +4484,7 @@ png_do_expand_palette(png_structrp png_p } row_info->bit_depth = 8; row_info->pixel_depth = 32; - row_info->rowbytes = row_width * 4; + row_info->rowbytes = (size_t)row_width * 4; row_info->color_type = 6; row_info->channels = 4; } @@ -4492,7 +4492,7 @@ png_do_expand_palette(png_structrp png_p else { sp = row + (size_t)row_width - 1; - dp = row + (size_t)(row_width * 3) - 1; + dp = row + (size_t)row_width * 3 - 1; i = 0; #ifdef PNG_ARM_NEON_INTRINSICS_AVAILABLE i = png_do_expand_palette_rgb8_neon(png_ptr, row_info, row, @@ -4511,7 +4511,7 @@ png_do_expand_palette(png_structrp png_p row_info->bit_depth = 8; row_info->pixel_depth = 24; - row_info->rowbytes = row_width * 3; + row_info->rowbytes = (size_t)row_width * 3; row_info->color_type = 2; row_info->channels = 3; } Index: pngset.c =================================================================== RCS file: /cvs/OpenBSD/xenocara/lib/libpng/pngset.c,v diff -u -p -u -r1.2 pngset.c --- pngset.c 26 Mar 2026 19:57:02 -0000 1.2 +++ pngset.c 9 Apr 2026 05:30:08 -0000 @@ -385,6 +385,7 @@ void PNGAPI png_set_hIST(png_const_structrp png_ptr, png_inforp info_ptr, png_const_uint_16p hist) { + png_uint_16 safe_hist[PNG_MAX_PALETTE_LENGTH]; int i; png_debug1(1, "in %s storage function", "hIST"); @@ -401,6 +402,13 @@ png_set_hIST(png_const_structrp png_ptr, return; } + /* Snapshot the caller's hist before freeing, in case it points to + * info_ptr->hist (getter-to-setter aliasing). + */ + memcpy(safe_hist, hist, (unsigned int)info_ptr->num_palette * + (sizeof (png_uint_16))); + hist = safe_hist; + png_free_data(png_ptr, info_ptr, PNG_FREE_HIST, 0); /* Changed from info->num_palette to PNG_MAX_PALETTE_LENGTH in @@ -742,7 +750,7 @@ void PNGAPI png_set_PLTE(png_structrp png_ptr, png_inforp info_ptr, png_const_colorp palette, int num_palette) { - + png_color safe_palette[PNG_MAX_PALETTE_LENGTH]; png_uint_32 max_palette_length; png_debug1(1, "in %s storage function", "PLTE"); @@ -776,6 +784,15 @@ png_set_PLTE(png_structrp png_ptr, png_i png_error(png_ptr, "Invalid palette"); } + /* Snapshot the caller's palette before freeing, in case it points to + * info_ptr->palette (getter-to-setter aliasing). + */ + if (num_palette > 0) + memcpy(safe_palette, palette, (unsigned int)num_palette * + (sizeof (png_color))); + + palette = safe_palette; + png_free_data(png_ptr, info_ptr, PNG_FREE_PLTE, 0); /* Changed in libpng-1.2.1 to allocate PNG_MAX_PALETTE_LENGTH instead @@ -937,6 +954,7 @@ png_set_text_2(png_const_structrp png_pt png_const_textp text_ptr, int num_text) { int i; + png_textp old_text = NULL; png_debug1(1, "in text storage function, chunk typeid = 0x%lx", png_ptr == NULL ? 0xabadca11UL : (unsigned long)png_ptr->chunk_name); @@ -984,7 +1002,10 @@ png_set_text_2(png_const_structrp png_pt return 1; } - png_free(png_ptr, info_ptr->text); + /* Defer freeing the old array until after the copy loop below, + * in case text_ptr aliases info_ptr->text (getter-to-setter). + */ + old_text = info_ptr->text; info_ptr->text = new_text; info_ptr->free_me |= PNG_FREE_TEXT; @@ -1069,6 +1090,7 @@ png_set_text_2(png_const_structrp png_pt { png_chunk_report(png_ptr, "text chunk: out of memory", PNG_CHUNK_WRITE_ERROR); + png_free(png_ptr, old_text); return 1; } @@ -1122,6 +1144,8 @@ png_set_text_2(png_const_structrp png_pt png_debug1(3, "transferred text chunk %d", info_ptr->num_text); } + png_free(png_ptr, old_text); + return 0; } #endif @@ -1165,6 +1189,16 @@ png_set_tRNS(png_structrp png_ptr, png_i if (trans_alpha != NULL) { + /* Snapshot the caller's trans_alpha before freeing, in case it + * points to info_ptr->trans_alpha (getter-to-setter aliasing). + */ + png_byte safe_trans[PNG_MAX_PALETTE_LENGTH]; + + if (num_trans > 0 && num_trans <= PNG_MAX_PALETTE_LENGTH) + memcpy(safe_trans, trans_alpha, (size_t)num_trans); + + trans_alpha = safe_trans; + png_free_data(png_ptr, info_ptr, PNG_FREE_TRNS, 0); if (num_trans > 0 && num_trans <= PNG_MAX_PALETTE_LENGTH) @@ -1249,6 +1283,7 @@ png_set_sPLT(png_const_structrp png_ptr, */ { png_sPLT_tp np; + png_sPLT_tp old_spalettes; png_debug1(1, "in %s storage function", "sPLT"); @@ -1269,7 +1304,10 @@ png_set_sPLT(png_const_structrp png_ptr, return; } - png_free(png_ptr, info_ptr->splt_palettes); + /* Defer freeing the old array until after the copy loop below, + * in case entries aliases info_ptr->splt_palettes (getter-to-setter). + */ + old_spalettes = info_ptr->splt_palettes; info_ptr->splt_palettes = np; info_ptr->free_me |= PNG_FREE_SPLT; @@ -1333,6 +1371,8 @@ png_set_sPLT(png_const_structrp png_ptr, } while (--nentries); + png_free(png_ptr, old_spalettes); + if (nentries > 0) png_chunk_report(png_ptr, "sPLT out of memory", PNG_CHUNK_WRITE_ERROR); } @@ -1381,6 +1421,7 @@ png_set_unknown_chunks(png_const_structr png_inforp info_ptr, png_const_unknown_chunkp unknowns, int num_unknowns) { png_unknown_chunkp np; + png_unknown_chunkp old_unknowns; if (png_ptr == NULL || info_ptr == NULL || num_unknowns <= 0 || unknowns == NULL) @@ -1427,7 +1468,10 @@ png_set_unknown_chunks(png_const_structr return; } - png_free(png_ptr, info_ptr->unknown_chunks); + /* Defer freeing the old array until after the copy loop below, + * in case unknowns aliases info_ptr->unknown_chunks (getter-to-setter). + */ + old_unknowns = info_ptr->unknown_chunks; info_ptr->unknown_chunks = np; /* safe because it is initialized */ info_ptr->free_me |= PNG_FREE_UNKN; @@ -1473,6 +1517,8 @@ png_set_unknown_chunks(png_const_structr ++np; ++(info_ptr->unknown_chunks_num); } + + png_free(png_ptr, old_unknowns); } void PNGAPI -- Matthieu Herrb