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. --- pngread.c | 11 +++++------ pngrtran.c | 8 +++++++- pngrutil.c | 13 ------------- pngset.c | 28 +++++++++++++++++++--------- pngwrite.c | 4 ++++ 5 files changed, 35 insertions(+), 29 deletions(-) Index: libpng-1.6.44/pngread.c =================================================================== --- libpng-1.6.44.orig/pngread.c +++ libpng-1.6.44/pngread.c @@ -963,12 +963,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) Index: libpng-1.6.44/pngrtran.c =================================================================== --- libpng-1.6.44.orig/pngrtran.c +++ libpng-1.6.44/pngrtran.c @@ -742,7 +742,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; Index: libpng-1.6.44/pngset.c =================================================================== --- libpng-1.6.44.orig/pngset.c +++ libpng-1.6.44/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; } Index: libpng-1.6.44/pngwrite.c =================================================================== --- libpng-1.6.44.orig/pngwrite.c +++ libpng-1.6.44/pngwrite.c @@ -983,6 +983,10 @@ png_write_destroy(png_structrp png_ptr) 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 * for how this happens.