|
| 1 | +From 788a624d7387a758ffd5c7ab010f1870dea753a1 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Cosmin Truta <ctruta@gmail.com> |
| 3 | +Date: Sat, 29 Nov 2025 00:39:16 +0200 |
| 4 | +Subject: [PATCH] Fix an out-of-bounds read in `png_image_read_composite` |
| 5 | + |
| 6 | +Add a defensive bounds check before calling PNG_sRGB_FROM_LINEAR to |
| 7 | +prevent reading up to 506 entries (1012 bytes) past `png_sRGB_base[]`. |
| 8 | + |
| 9 | +For palette images with gamma, `png_init_read_transformations` |
| 10 | +clears PNG_COMPOSE after compositing on the palette, but it leaves |
| 11 | +PNG_FLAG_OPTIMIZE_ALPHA set. The simplified API then calls |
| 12 | +`png_image_read_composite` with sRGB data (not linear premultiplied), |
| 13 | +causing the index to reach 1017. (The maximum valid index is 511.) |
| 14 | + |
| 15 | +NOTE: |
| 16 | +This is a defensive fix that addresses the security issue (out-of-bounds |
| 17 | +read) but *NOT* the correctness issue (wrong output). When the clamp |
| 18 | +triggers, the affected pixels are clamped to white instead of the |
| 19 | +correct composited color. Valid PNG images may render incorrectly with |
| 20 | +the simplified API. |
| 21 | + |
| 22 | +TODO: |
| 23 | +We already know the root cause is a flag synchronization error. |
| 24 | +For palette images with gamma, `png_init_read_transformations` |
| 25 | +clears PNG_COMPOSE but leaves PNG_FLAG_OPTIMIZE_ALPHA set, causing |
| 26 | +`png_image_read_composite` to misinterpret sRGB data as linear |
| 27 | +premultiplied. However, we have yet to implement an architectural fix |
| 28 | +that requires coordinating the simplified API with the transformation |
| 29 | +pipeline. |
| 30 | + |
| 31 | +Reported-by: flyfish101 <flyfish101@users.noreply.github.com> |
| 32 | +Upstream Reference: https://github.com/pnggroup/libpng/commit/788a624d7387a758ffd5c7ab010f1870dea753a1.patch & https://github.com/pnggroup/libpng/commit/a05a48b756de63e3234ea6b3b938b8f5f862484a.patch |
| 33 | + |
| 34 | +--- |
| 35 | + src/3rdparty/libpng/pngread.c | 54 +++++++++++++++++++++++++++++----- |
| 36 | + src/3rdparty/libpng/pngrtran.c | 1 + |
| 37 | + 2 files changed, 47 insertions(+), 8 deletions(-) |
| 38 | + |
| 39 | +diff --git a/src/3rdparty/libpng/pngread.c b/src/3rdparty/libpng/pngread.c |
| 40 | +index 07a39df6..85958188 100644 |
| 41 | +--- a/src/3rdparty/libpng/pngread.c |
| 42 | ++++ b/src/3rdparty/libpng/pngread.c |
| 43 | +@@ -3296,6 +3296,7 @@ png_image_read_composite(png_voidp argument) |
| 44 | + ptrdiff_t step_row = display->row_bytes; |
| 45 | + unsigned int channels = |
| 46 | + (image->format & PNG_FORMAT_FLAG_COLOR) != 0 ? 3 : 1; |
| 47 | ++ int optimize_alpha = (png_ptr->flags & PNG_FLAG_OPTIMIZE_ALPHA) != 0; |
| 48 | + int pass; |
| 49 | + |
| 50 | + for (pass = 0; pass < passes; ++pass) |
| 51 | +@@ -3504,14 +3505,51 @@ png_image_read_background(png_voidp argument) |
| 52 | + |
| 53 | + if (alpha < 255) /* else just use component */ |
| 54 | + { |
| 55 | +- /* Since PNG_OPTIMIZED_ALPHA was not set it is |
| 56 | +- * necessary to invert the sRGB transfer |
| 57 | +- * function and multiply the alpha out. |
| 58 | +- */ |
| 59 | +- component = png_sRGB_table[component] * alpha; |
| 60 | +- component += png_sRGB_table[outrow[0]] * |
| 61 | +- (255-alpha); |
| 62 | +- component = PNG_sRGB_FROM_LINEAR(component); |
| 63 | ++ if (optimize_alpha != 0) |
| 64 | ++ { |
| 65 | ++ /* This is PNG_OPTIMIZED_ALPHA, the component value |
| 66 | ++ * is a linear 8-bit value. Combine this with the |
| 67 | ++ * current outrow[c] value which is sRGB encoded. |
| 68 | ++ * Arithmetic here is 16-bits to preserve the output |
| 69 | ++ * values correctly. |
| 70 | ++ */ |
| 71 | ++ component *= 257*255; /* =65535 */ |
| 72 | ++ component += (255-alpha)*png_sRGB_table[outrow[c]]; |
| 73 | ++ |
| 74 | ++ /* Clamp to the valid range to defend against |
| 75 | ++ * unforeseen cases where the data might be sRGB |
| 76 | ++ * instead of linear premultiplied. |
| 77 | ++ * (Belt-and-suspenders for GitHub Issue #764.) |
| 78 | ++ */ |
| 79 | ++ |
| 80 | ++ if (component > 255*65535) |
| 81 | ++ component = 255*65535; |
| 82 | ++ |
| 83 | ++ /* So 'component' is scaled by 255*65535 and is |
| 84 | ++ * therefore appropriate for the sRGB-to-linear |
| 85 | ++ * conversion table. Clamp to the valid range |
| 86 | ++ * as a defensive measure against an internal |
| 87 | ++ * libpng bug where the data is sRGB rather than |
| 88 | ++ * linear premultiplied. |
| 89 | ++ */ |
| 90 | ++ if (component > 255*65535) |
| 91 | ++ component = 255*65535; |
| 92 | ++ component = PNG_sRGB_FROM_LINEAR(component); |
| 93 | ++ } |
| 94 | ++ else |
| 95 | ++ { |
| 96 | ++ /* Compositing was already done on the palette |
| 97 | ++ * entries. The data is sRGB premultiplied on black. |
| 98 | ++ * Composite with the background in sRGB space. |
| 99 | ++ * This is not gamma-correct, but matches what was |
| 100 | ++ * done to the palette. |
| 101 | ++ */ |
| 102 | ++ png_uint_32 background = outrow[c]; |
| 103 | ++ component += ((255-alpha) * background + 127) / 255; |
| 104 | ++ if (component > 255) |
| 105 | ++ component = 255; |
| 106 | ++ } |
| 107 | ++ |
| 108 | + } |
| 109 | + |
| 110 | + outrow[0] = (png_byte)component; |
| 111 | +diff --git a/src/3rdparty/libpng/pngrtran.c b/src/3rdparty/libpng/pngrtran.c |
| 112 | +index 1526123e..e329a715 100644 |
| 113 | +--- a/src/3rdparty/libpng/pngrtran.c |
| 114 | ++++ b/src/3rdparty/libpng/pngrtran.c |
| 115 | +@@ -1720,6 +1720,7 @@ png_init_read_transformations(png_structrp png_ptr) |
| 116 | + * transformations elsewhere. |
| 117 | + */ |
| 118 | + png_ptr->transformations &= ~(PNG_COMPOSE | PNG_GAMMA); |
| 119 | ++ png_ptr->flags &= ~PNG_FLAG_OPTIMIZE_ALPHA; |
| 120 | + } /* color_type == PNG_COLOR_TYPE_PALETTE */ |
| 121 | + |
| 122 | + /* if (png_ptr->background_gamma_type!=PNG_BACKGROUND_GAMMA_UNKNOWN) */ |
| 123 | +-- |
| 124 | +2.45.4 |
| 125 | + |
0 commit comments