From 7ea9eea884a2328cc7fdcb3c0c00246a50d90667 Mon Sep 17 00:00:00 2001 From: Cosmin Truta Date: Fri, 20 Mar 2026 17:37:22 +0200 Subject: [PATCH] fix: Resolve use-after-free on `png_ptr->palette` Give `png_struct` its own independently-allocated copy of the palette buffer, decoupling it from `info_struct`'s palette. Allocate both copies with `png_calloc` to zero-fill, because the ARM NEON palette riffle reads all 256 entries unconditionally. In function `png_set_PLTE`, `png_ptr->palette` was aliased directly to `info_ptr->palette`: a single heap buffer shared across two structs with independent lifetimes. If the buffer was freed through `info_ptr` (via `png_free_data(PNG_FREE_PLTE)` or a second call to `png_set_PLTE`), `png_ptr->palette` became a dangling pointer. Subsequent row reads, performed in `png_do_expand_palette` and in other transform functions, dereferenced (and in the bit-shift path, wrote to) freed memory. Also fix `png_set_quantize` to allocate an owned copy of the caller's palette rather than aliasing the user pointer, so that the unconditional free in `png_read_destroy` does not free unmanaged memory. CVE: CVE-2026-33416 Upstream-Status: Backport [https://github.com/pnggroup/libpng/commit/7ea9eea884a2328cc7fdcb3c0c00246a50d90667] Comment: Refreshed hunk to match latest scarthgap Signed-off-by: Sourav Kumar Pramanik Signed-off-by: Zahir Hussain --- pngread.c | 11 +++++------ pngrtran.c | 8 +++++++- pngrutil.c | 13 ------------- pngset.c | 28 +++++++++++++++++++--------- pngwrite.c | 4 ++++ 5 files changed, 35 insertions(+), 29 deletions(-) diff --git a/pngread.c b/pngread.c index 0086edf6cf..e1d38d578a 100644 --- a/pngread.c +++ b/pngread.c @@ -959,12 +959,11 @@ png_read_destroy(png_structrp png_ptr) png_ptr->quantize_index = NULL; #endif - if ((png_ptr->free_me & PNG_FREE_PLTE) != 0) - { - png_zfree(png_ptr, png_ptr->palette); - png_ptr->palette = NULL; - } - png_ptr->free_me &= ~PNG_FREE_PLTE; + /* png_ptr->palette is always independently allocated (not aliased + * with info_ptr->palette), so free it unconditionally. + */ + png_free(png_ptr, png_ptr->palette); + png_ptr->palette = NULL; #if defined(PNG_tRNS_SUPPORTED) || \ defined(PNG_READ_EXPAND_SUPPORTED) || defined(PNG_READ_BACKGROUND_SUPPORTED) diff --git a/pngrtran.c b/pngrtran.c index bfb7d423b7..fd736ab672 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -750,7 +750,13 @@ png_set_quantize(png_structrp png_ptr, p } if (png_ptr->palette == NULL) { - png_ptr->palette = palette; + /* Allocate an owned copy rather than aliasing the caller's pointer, + * so that png_read_destroy can free png_ptr->palette unconditionally. + */ + png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, + PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); + memcpy(png_ptr->palette, palette, (unsigned int)num_palette * + (sizeof (png_color))); } png_ptr->num_palette = (png_uint_16)num_palette; diff --git a/pngrutil.c b/pngrutil.c index a19507bf1b..3a35fe9de2 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -1047,14 +1047,6 @@ png_handle_PLTE(png_structrp png_ptr, pn } #endif - /* 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); /* The three chunks, bKGD, hIST and tRNS *must* appear after PLTE and before diff --git a/pngset.c b/pngset.c index dccc6498d7..b9ccb7fb15 100644 --- a/pngset.c +++ b/pngset.c @@ -595,28 +595,38 @@ png_set_PLTE(png_structrp png_ptr, png_i png_error(png_ptr, "Invalid palette"); } - /* 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. + * + * Allocate independent buffers for info_ptr and png_ptr so that the + * lifetime of png_ptr->palette is decoupled from the lifetime of + * info_ptr->palette. Previously, these two pointers were aliased, + * which caused a use-after-free vulnerability if png_free_data freed + * info_ptr->palette while png_ptr->palette was still in use by the + * row transform functions (e.g. png_do_expand_palette). + * + * Both buffers are allocated with png_calloc to zero-fill, because + * the ARM NEON palette riffle reads all 256 entries unconditionally, + * regardless of num_palette. */ + png_free(png_ptr, png_ptr->palette); png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); + info_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, + PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); + png_ptr->num_palette = info_ptr->num_palette = (png_uint_16)num_palette; if (num_palette > 0) + { + memcpy(info_ptr->palette, palette, (unsigned int)num_palette * + (sizeof (png_color))); memcpy(png_ptr->palette, palette, (unsigned int)num_palette * (sizeof (png_color))); + } - info_ptr->palette = png_ptr->palette; - info_ptr->num_palette = png_ptr->num_palette = (png_uint_16)num_palette; info_ptr->free_me |= PNG_FREE_PLTE; info_ptr->valid |= PNG_INFO_PLTE; } diff --git a/pngwrite.c b/pngwrite.c index 84af1e73fb..348763e940 100644 --- a/pngwrite.c +++ b/pngwrite.c @@ -982,6 +982,10 @@ png_write_destroy(png_structrp png_ptr) png_free(png_ptr, png_ptr->trans_alpha); png_ptr->trans_alpha = NULL; #endif + + /* Free the independent copy of the palette owned by png_struct. */ + png_free(png_ptr, png_ptr->palette); + png_ptr->palette = NULL; /* The error handling and memory handling information is left intact at this * point: the jmp_buf may still have to be freed. See png_destroy_png_struct