Index | Thread | Search

From:
Matthieu Herrb <matthieu@openbsd.org>
Subject:
Re: [update] png to 1.6.57
To:
Theo de Raadt <deraadt@openbsd.org>
Cc:
Theo Buehler <tb@theobuehler.org>, Matthieu Herrb <matthieu@openbsd.org>, ports@openbsd.org
Date:
Thu, 9 Apr 2026 07:37:41 +0200

Download raw body.

Thread
On Wed, Apr 08, 2026 at 11:29:17PM -0600, Theo de Raadt wrote:
> Theo Buehler <tb@theobuehler.org> 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