From 218612ddd6b17944e21eda56caf8b4bf7779d1ea Mon Sep 17 00:00:00 2001 From: Cosmin Truta Date: Wed, 19 Nov 2025 21:45:13 +0200 Subject: [PATCH] Rearchitect the fix to the buffer overflow in `png_image_finish_read` Undo the fix from commit 16b5e3823918840aae65c0a6da57c78a5a496a4d. That fix turned out to be unnecessarily limiting. It rejected all 16-to-8 bit transformations, although the vulnerability only affects interlaced PNGs where `png_combine_row` writes using IHDR bit-depth before the transformation completes. The proper solution is to add an intermediate `local_row` buffer, specifically for the slow but necessary step of 16-to-8 bit conversion of interlaced images. (The processing of non-interlaced images remains intact, using the fast path.) We added the flag `do_local_scale` and the function `png_image_read_direct_scaled`, following the pattern that involves `do_local_compose`. In conclusion: - The 16-to-8 bit transformations of interlaced images are now safe, as they use an intermediate buffer. - The 16-to-8 bit transformations of non-interlaced images remain safe, as the fast path remains unchanged. - All our regression tests are now passing. CVE: CVE-2025-65018 Upstream-Status: Backport [https://github.com/pnggroup/libpng/commit/218612ddd6b17944e21eda56caf8b4bf7779d1ea] Signed-off-by: Peter Marko --- pngread.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/pngread.c b/pngread.c index 92571ec33..79917daaa 100644 --- a/pngread.c +++ b/pngread.c @@ -3260,6 +3260,54 @@ png_image_read_colormapped(png_voidp argument) } } +/* Row reading for interlaced 16-to-8 bit depth conversion with local buffer. */ +static int +png_image_read_direct_scaled(png_voidp argument) +{ + png_image_read_control *display = png_voidcast(png_image_read_control*, + argument); + png_imagep image = display->image; + png_structrp png_ptr = image->opaque->png_ptr; + png_bytep local_row = png_voidcast(png_bytep, display->local_row); + png_bytep first_row = png_voidcast(png_bytep, display->first_row); + ptrdiff_t row_bytes = display->row_bytes; + int passes; + + /* Handle interlacing. */ + switch (png_ptr->interlaced) + { + case PNG_INTERLACE_NONE: + passes = 1; + break; + + case PNG_INTERLACE_ADAM7: + passes = PNG_INTERLACE_ADAM7_PASSES; + break; + + default: + png_error(png_ptr, "unknown interlace type"); + } + + /* Read each pass using local_row as intermediate buffer. */ + while (--passes >= 0) + { + png_uint_32 y = image->height; + png_bytep output_row = first_row; + + for (; y > 0; --y) + { + /* Read into local_row (gets transformed 8-bit data). */ + png_read_row(png_ptr, local_row, NULL); + + /* Copy from local_row to user buffer. */ + memcpy(output_row, local_row, (size_t)row_bytes); + output_row += row_bytes; + } + } + + return 1; +} + /* Just the row reading part of png_image_read. */ static int png_image_read_composite(png_voidp argument) @@ -3678,6 +3726,7 @@ png_image_read_direct(png_voidp argument) int linear = (format & PNG_FORMAT_FLAG_LINEAR) != 0; int do_local_compose = 0; int do_local_background = 0; /* to avoid double gamma correction bug */ + int do_local_scale = 0; /* for interlaced 16-to-8 bit conversion */ int passes = 0; /* Add transforms to ensure the correct output format is produced then check @@ -3804,8 +3853,16 @@ png_image_read_direct(png_voidp argument) png_set_expand_16(png_ptr); else /* 8-bit output */ + { png_set_scale_16(png_ptr); + /* For interlaced images, use local_row buffer to avoid overflow + * in png_combine_row() which writes using IHDR bit-depth. + */ + if (png_ptr->interlaced != 0) + do_local_scale = 1; + } + change &= ~PNG_FORMAT_FLAG_LINEAR; } @@ -4081,6 +4138,24 @@ png_image_read_direct(png_voidp argument) return result; } + else if (do_local_scale != 0) + { + /* For interlaced 16-to-8 conversion, use an intermediate row buffer + * to avoid buffer overflows in png_combine_row. The local_row is sized + * for the transformed (8-bit) output, preventing the overflow that would + * occur if png_combine_row wrote 16-bit data directly to the user buffer. + */ + int result; + png_voidp row = png_malloc(png_ptr, png_get_rowbytes(png_ptr, info_ptr)); + + display->local_row = row; + result = png_safe_execute(image, png_image_read_direct_scaled, display); + display->local_row = NULL; + png_free(png_ptr, row); + + return result; + } + else { png_alloc_size_t row_bytes = (png_alloc_size_t)display->row_bytes; @@ -4164,20 +4239,6 @@ png_image_finish_read(png_imagep image, png_const_colorp background, int result; png_image_read_control display; - /* Reject bit depth mismatches to avoid buffer overflows. */ - png_uint_32 ihdr_bit_depth = - image->opaque->png_ptr->bit_depth; - int requested_linear = - (image->format & PNG_FORMAT_FLAG_LINEAR) != 0; - if (ihdr_bit_depth == 16 && !requested_linear) - return png_image_error(image, - "png_image_finish_read: " - "16-bit PNG must use 16-bit output format"); - if (ihdr_bit_depth < 16 && requested_linear) - return png_image_error(image, - "png_image_finish_read: " - "8-bit PNG must not use 16-bit output format"); - memset(&display, 0, (sizeof display)); display.image = image; display.buffer = buffer;