public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
From: "Yoann Congal" <yoann.congal@smile.fr>
To: <pahaditechie@gmail.com>, <openembedded-core@lists.openembedded.org>
Cc: <ashissh7@cisco.com>
Subject: Re: [OE-core][scarthgap][PATCH] libpng: Fix CVE-2026-33416
Date: Thu, 23 Apr 2026 15:45:38 +0200	[thread overview]
Message-ID: <DI0KW340PLYT.2HO6B3DWHUUGT@smile.fr> (raw)
In-Reply-To: <20260406063250.3479054-1-pahaditechie@gmail.com>

Hello,

Thanks for the patch. A couple of remarks, questions:

On Mon Apr 6, 2026 at 8:32 AM CEST, Ashish Sharma via lists.openembedded.org wrote:
> Backports upstream fix for use-after-free vulnerability in
> png_set_tRNS() where png_ptr->trans_alpha was aliased to
> info_ptr->trans_alpha. If png_free_data() freed the info_ptr buffer,
> png_ptr held a dangling pointer causing UAF in png_do_expand_palette().
>
> Fix gives png_struct its own independent allocation, decoupling
> the two lifetimes.
>
> CVE: CVE-2026-33416
> CVSS: 7.5 (HIGH)
^ Those 2 lines are not needed here (but you can keep themp if you
want).

You need to provide a justification for the CVE patch. What makes you
think this is the right patch for the CVE? Usually a link to the NVD or
the debian security tracker is enough (provided that they mention you
upstream commit).



> Signed-off-by: Ashish Sharma <pahaditechie@gmail.com>
> ---


>  .../libpng/files/CVE-2026-33416.patch         | 143 ++++++++++++++++++

In that case, the NVD and Debian Security Tracker mention 4 patches to
fix this CVE:
  NVD Tracker:    https://nvd.nist.gov/vuln/detail/CVE-2026-33416
  Debian Tracker: https://security-tracker.debian.org/tracker/CVE-2026-33416
    Fix URL:      https://github.com/pnggroup/libpng/commit/23019269764e35ed8458e517f1897bd3c54820eb
    Fix URL:      https://github.com/pnggroup/libpng/commit/a3a21443ed12bfa1ef46fa0d4fb2b74a0fa34a25
    Fix URL:      https://github.com/pnggroup/libpng/commit/7ea9eea884a2328cc7fdcb3c0c00246a50d90667
    Fix URL:      https://github.com/pnggroup/libpng/commit/c1b0318b393c90679e6fa5bc1d329fd5d5012ec1
... But you only applied one. Why?

Can you send a v2 with those remarks addressed?

Regards,

>  .../libpng/libpng_1.6.42.bb                   |   1 +
>  2 files changed, 144 insertions(+)
>  create mode 100644 meta/recipes-multimedia/libpng/files/CVE-2026-33416.patch
>
> diff --git a/meta/recipes-multimedia/libpng/files/CVE-2026-33416.patch b/meta/recipes-multimedia/libpng/files/CVE-2026-33416.patch
> new file mode 100644
> index 0000000000..c563d977e3
> --- /dev/null
> +++ b/meta/recipes-multimedia/libpng/files/CVE-2026-33416.patch
> @@ -0,0 +1,143 @@
> +From 7c1c160358b839bd177d1134acede9891e256027 Mon Sep 17 00:00:00 2001
> +From: Oblivionsage <cookieandcream560@gmail.com>
> +Date: Sun, 15 Mar 2026 10:35:29 +0100
> +Subject: [PATCH] fix: Resolve use-after-free on `png_ptr->trans_alpha`
> +
> +The function `png_set_tRNS` sets `png_ptr->trans_alpha` to point at
> +`info_ptr->trans_alpha` directly, so both structs share the same heap
> +buffer. If the application calls `png_free_data(PNG_FREE_TRNS)`, or if
> +`png_set_tRNS` is called a second time, the buffer is freed through
> +`info_ptr` while `png_ptr` still holds a dangling reference. Any
> +subsequent row read that hits the function `png_do_expand_palette` will
> +dereference freed memory.
> +
> +The fix gives `png_struct` its own allocation instead of aliasing the
> +`info_ptr` pointer. This was already flagged with a TODO in
> +`png_handle_tRNS` ("horrible side effect ... Fix this.") but it was
> +never addressed.
> +
> +Verified with AddressSanitizer. All 34 existing tests pass without
> +regressions.
> +
> +CVE: CVE-2026-33416
> +Upstream-Status: Backport [https://github.com/pnggroup/libpng/commit/23019269764e35ed8458e517f1897bd3c54820eb]
> +
> +Reviewed-by: Cosmin Truta <ctruta@gmail.com>
> +Signed-off-by: Cosmin Truta <ctruta@gmail.com>
> +(cherry picked from commit 23019269764e35ed8458e517f1897bd3c54820eb)
> +Signed-off-by: Ashish Sharma <pahaditechie@gmail.com>
> +---
> + pngread.c  | 11 +++++------
> + pngrutil.c |  4 ----
> + pngset.c   | 31 +++++++++++++++++++------------
> + pngwrite.c |  6 ++++++
> + 4 files changed, 30 insertions(+), 22 deletions(-)
> +
> +diff --git a/pngread.c b/pngread.c
> +index 008a41856..b8a64a6e7 100644
> +--- a/pngread.c
> ++++ b/pngread.c
> +@@ -968,12 +968,11 @@ png_read_destroy(png_structrp png_ptr)
> + 
> + #if defined(PNG_tRNS_SUPPORTED) || \
> +     defined(PNG_READ_EXPAND_SUPPORTED) || defined(PNG_READ_BACKGROUND_SUPPORTED)
> +-   if ((png_ptr->free_me & PNG_FREE_TRNS) != 0)
> +-   {
> +-      png_free(png_ptr, png_ptr->trans_alpha);
> +-      png_ptr->trans_alpha = NULL;
> +-   }
> +-   png_ptr->free_me &= ~PNG_FREE_TRNS;
> ++   /* png_ptr->trans_alpha is always independently allocated (not aliased
> ++    * with info_ptr->trans_alpha), so free it unconditionally.
> ++    */
> ++   png_free(png_ptr, png_ptr->trans_alpha);
> ++   png_ptr->trans_alpha = NULL;
> + #endif
> + 
> +    inflateEnd(&png_ptr->zstream);
> +diff --git a/pngrutil.c b/pngrutil.c
> +index d31dc21da..2128b2a66 100644
> +--- a/pngrutil.c
> ++++ b/pngrutil.c
> +@@ -1905,10 +1905,6 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr, png_uint_32 length)
> +       return;
> +    }
> + 
> +-   /* TODO: this is a horrible side effect in the palette case because the
> +-    * png_struct ends up with a pointer to the tRNS buffer owned by the
> +-    * png_info.  Fix this.
> +-    */
> +    png_set_tRNS(png_ptr, info_ptr, readbuf, png_ptr->num_trans,
> +        &(png_ptr->trans_color));
> + }
> +diff --git a/pngset.c b/pngset.c
> +index eb1c8c7a3..3ae16f4e1 100644
> +--- a/pngset.c
> ++++ b/pngset.c
> +@@ -990,28 +990,35 @@ png_set_tRNS(png_structrp png_ptr, png_inforp info_ptr,
> + 
> +    if (trans_alpha != NULL)
> +    {
> +-       /* It may not actually be necessary to set png_ptr->trans_alpha 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 is incorrect; png_handle_tRNS effectively
> +-        * relies on png_set_tRNS storing the information in png_struct
> +-        * (otherwise it won't be there for the code in pngrtran.c).
> +-        */
> +-
> +        png_free_data(png_ptr, info_ptr, PNG_FREE_TRNS, 0);
> + 
> +        if (num_trans > 0 && num_trans <= PNG_MAX_PALETTE_LENGTH)
> +        {
> +-         /* Changed from num_trans to PNG_MAX_PALETTE_LENGTH in version 1.2.1 */
> ++          /* Allocate info_ptr's copy of the transparency data. */
> +           info_ptr->trans_alpha = png_voidcast(png_bytep,
> +               png_malloc(png_ptr, PNG_MAX_PALETTE_LENGTH));
> +           memcpy(info_ptr->trans_alpha, trans_alpha, (size_t)num_trans);
> +-
> +           info_ptr->free_me |= PNG_FREE_TRNS;
> +           info_ptr->valid |= PNG_INFO_tRNS;
> ++
> ++          /* Allocate an independent copy for png_struct, so that the
> ++           * lifetime of png_ptr->trans_alpha is decoupled from the
> ++           * lifetime of info_ptr->trans_alpha.  Previously these two
> ++           * pointers were aliased, which caused a use-after-free if
> ++           * png_free_data freed info_ptr->trans_alpha while
> ++           * png_ptr->trans_alpha was still in use by the row transform
> ++           * functions (e.g. png_do_expand_palette).
> ++           */
> ++          png_free(png_ptr, png_ptr->trans_alpha);
> ++          png_ptr->trans_alpha = png_voidcast(png_bytep,
> ++              png_malloc(png_ptr, PNG_MAX_PALETTE_LENGTH));
> ++          memcpy(png_ptr->trans_alpha, trans_alpha, (size_t)num_trans);
> ++       }
> ++       else
> ++       {
> ++          png_free(png_ptr, png_ptr->trans_alpha);
> ++          png_ptr->trans_alpha = NULL;
> +        }
> +-       png_ptr->trans_alpha = info_ptr->trans_alpha;
> +    }
> + 
> +    if (trans_color != NULL)
> +diff --git a/pngwrite.c b/pngwrite.c
> +index 77e412f43..e4e2973f8 100644
> +--- a/pngwrite.c
> ++++ b/pngwrite.c
> +@@ -977,6 +977,12 @@ png_write_destroy(png_structrp png_ptr)
> +    png_ptr->chunk_list = NULL;
> + #endif
> + 
> ++#if defined(PNG_tRNS_SUPPORTED)
> ++   /* Free the independent copy of trans_alpha owned by png_struct. */
> ++   png_free(png_ptr, png_ptr->trans_alpha);
> ++   png_ptr->trans_alpha = NULL;
> ++#endif
> ++
> +    /* 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.
> +-- 
> +2.44.4
> diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.42.bb b/meta/recipes-multimedia/libpng/libpng_1.6.42.bb
> index 7471315fdd..4d8be5d843 100644
> --- a/meta/recipes-multimedia/libpng/libpng_1.6.42.bb
> +++ b/meta/recipes-multimedia/libpng/libpng_1.6.42.bb
> @@ -24,6 +24,7 @@ SRC_URI = "${SOURCEFORGE_MIRROR}/project/${BPN}/${BPN}${LIBV}/${PV}/${BP}.tar.xz
>             file://CVE-2026-22695.patch \
>             file://CVE-2026-22801.patch \
>             file://CVE-2026-25646.patch \
> +           file://CVE-2026-33416.patch \
>  "
>  
>  SRC_URI[sha256sum] = "c919dbc11f4c03b05aba3f8884d8eb7adfe3572ad228af972bb60057bdb48450"


-- 
Yoann Congal
Smile ECS



      reply	other threads:[~2026-04-23 13:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-06  6:32 [OE-core][scarthgap][PATCH] libpng: Fix CVE-2026-33416 Ashish Sharma
2026-04-23 13:45 ` Yoann Congal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DI0KW340PLYT.2HO6B3DWHUUGT@smile.fr \
    --to=yoann.congal@smile.fr \
    --cc=ashissh7@cisco.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=pahaditechie@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox