From 2af2568484b0c37983b5c9cfbdac77e7f2e543d1 Mon Sep 17 00:00:00 2001 From: VAMSI KRISHNA <189184574+17krishna8@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:00:37 +0000 Subject: [PATCH 1/3] fix: Correct contradictory fcTL length bounds in read_chunks table The CDfcTL entry declared max_length=25 with min_length=26, which no chunk length can satisfy: png_handle_chunk rejects length < 26 as "too short" and everything else as "too long". An fcTL chunk is exactly 26 bytes (4-byte sequence number + 22 bytes of frame data), so both bounds must be 26, matching the CDacTL pattern where an acTL is exactly 8 bytes. The entry is currently unreachable because the APNG handlers are #defined to NULL and such chunks are routed to unknown-chunk handling, but the table must stay correct so that enabling native table-driven APNG dispatch does not silently reject every valid fcTL. --- pngrutil.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pngrutil.c b/pngrutil.c index 5dd5361671..cff37dc4e9 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -3232,7 +3232,14 @@ read_chunks[PNG_INDEX_unknown] = /* Allocates 'length+1'; checked in the handler */ # define CDtIME 7U, 7U, 0, hIHDR, 0 # define CDacTL 8U, 8U, hIDAT, hIHDR, 0 -# define CDfcTL 25U, 26U, 0, hIHDR, 1 +# define CDfcTL 26U, 26U, 0, hIHDR, 1 + /* The minimum and maximum fcTL lengths are equal because an fcTL is + * exactly 26 bytes long: a 4-byte big-endian sequence number followed by + * 22 bytes of frame data. The earlier value of 25U for max_length was + * inconsistent with min_length=26U and would have rejected every valid + * fcTL if this table entry were ever consulted (the handler is currently + * compiled out; see the '#define png_handle_fcTL NULL' above). + */ # define CDfdAT Limit, 4U, hIDAT, hIHDR, 1 /* Supported chunks from PNG extensions 1.5.0, NYI so limit */ # define CDoFFs 9U, 9U, hIDAT, hIHDR, 0 From 20939ec674ca8d9f37677190a7a445be14d86a08 Mon Sep 17 00:00:00 2001 From: VAMSI KRISHNA <189184574+17krishna8@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:00:52 +0000 Subject: [PATCH 2/3] fix: Report a rejected fcTL accurately in the progressive reader When png_handle_fcTL declined an fcTL chunk (for example an oversized frame description for the leading frame), the progressive reader reported "Missing required fcTL chunk in APNG stream" even though the fcTL was present in the stream; the misleading message sent users hunting for a nonexistent missing-chunk problem. The sequential reader already aborts such streams with the accurate "Out-of-order sequence number"/"Oversized frame in fcTL" diagnostics produced by the handler itself, so only the follow-up message here needs to describe the actual situation: the chunk was seen but not accepted. --- pngpread.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pngpread.c b/pngpread.c index 978559adeb..71a2584394 100644 --- a/pngpread.c +++ b/pngpread.c @@ -269,10 +269,16 @@ png_push_read_chunk(png_struct *png_ptr, png_info *info_ptr) png_read_reset(png_ptr); png_ptr->mode &= ~PNG_HAVE_fcTL; - png_handle_fcTL(png_ptr, info_ptr, png_ptr->push_length); - - if (!(png_ptr->mode & PNG_HAVE_fcTL)) - png_error(png_ptr, "Missing required fcTL chunk in APNG stream"); + png_handle_fcTL(png_ptr, info_ptr, png_ptr->push_length); + + /* We only get here for an fcTL chunk, so if PNG_HAVE_fcTL was not + * set above the chunk was present but rejected (e.g. an oversized + * or misplaced frame description); say that rather than claiming + * that the chunk is missing. + */ + if (!(png_ptr->mode & PNG_HAVE_fcTL)) + png_error(png_ptr, + "Invalid or misplaced fcTL chunk in APNG stream"); png_read_reinit(png_ptr, info_ptr); png_progressive_read_reset(png_ptr); From 558bdeab6db1910003b40adddf1ee7f172118766 Mon Sep 17 00:00:00 2001 From: VAMSI KRISHNA <189184574+17krishna8@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:49:39 +0000 Subject: [PATCH 3/3] refactor: Extract the maximum transformed pixel depth calculation png_read_start_row() open-codes the computation of the maximum pixel depth that a row can have after all enabled read transformations. The accompanying warning notes that png_read_transform_info (pngrtran.c) performs similar calculations and that the two must agree, otherwise memory overwrites become possible; the TODO asks for this to be fixed. As a first step, move the computation into its own function, png_read_max_pixel_depth(), so that: - the logic exists in exactly one place within pngrutil.c, - the invariant it maintains is documented next to the code, - follow-up work can share or assert on this value instead of re-deriving it. No functional change; behavior is identical for all configurations. --- pngrutil.c | 78 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 29 deletions(-) diff --git a/pngrutil.c b/pngrutil.c index cff37dc4e9..35d05f6c60 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -4597,37 +4597,17 @@ png_read_finish_row(png_struct *png_ptr) } #endif /* SEQUENTIAL_READ */ -void /* PRIVATE */ -png_read_start_row(png_struct *png_ptr) +/* Compute the maximum pixel depth that any row could have after all of + * the enabled read transformations. This was previously open-coded + * here only; keeping it in one place makes it possible to reason about + * (and assert on) consistency with png_do_read_transformations and + * png_read_transform_info, which must agree with this value - see the + * warning in png_read_start_row. + */ +static unsigned int +png_read_max_pixel_depth(png_struct *png_ptr) { unsigned int max_pixel_depth; - size_t row_bytes; - - png_debug(1, "in png_read_start_row"); - -#ifdef PNG_READ_TRANSFORMS_SUPPORTED - png_init_read_transformations(png_ptr); -#endif - if (png_ptr->interlaced != 0) - { - if ((png_ptr->transformations & PNG_INTERLACE) == 0) - png_ptr->num_rows = (png_ptr->height + png_pass_yinc[0] - 1 - - png_pass_ystart[0]) / png_pass_yinc[0]; - - else - png_ptr->num_rows = png_ptr->height; - - png_ptr->iwidth = (png_ptr->width + - png_pass_inc[png_ptr->pass] - 1 - - png_pass_start[png_ptr->pass]) / - png_pass_inc[png_ptr->pass]; - } - - else - { - png_ptr->num_rows = png_ptr->height; - png_ptr->iwidth = png_ptr->width; - } max_pixel_depth = (unsigned int)png_ptr->pixel_depth; @@ -4772,6 +4752,46 @@ defined(PNG_USER_TRANSFORM_PTR_SUPPORTED) } #endif + return max_pixel_depth; +} + +void /* PRIVATE */ +png_read_start_row(png_struct *png_ptr) +{ + unsigned int max_pixel_depth; + size_t row_bytes; + + png_debug(1, "in png_read_start_row"); + +#ifdef PNG_READ_TRANSFORMS_SUPPORTED + png_init_read_transformations(png_ptr); +#endif + if (png_ptr->interlaced != 0) + { + if ((png_ptr->transformations & PNG_INTERLACE) == 0) + png_ptr->num_rows = (png_ptr->height + png_pass_yinc[0] - 1 - + png_pass_ystart[0]) / png_pass_yinc[0]; + + else + png_ptr->num_rows = png_ptr->height; + + png_ptr->iwidth = (png_ptr->width + + png_pass_inc[png_ptr->pass] - 1 - + png_pass_start[png_ptr->pass]) / + png_pass_inc[png_ptr->pass]; + } + + else + { + png_ptr->num_rows = png_ptr->height; + png_ptr->iwidth = png_ptr->width; + } + + /* Calculate the maximum transformed pixel depth (see the warning + * above png_read_max_pixel_depth). + */ + max_pixel_depth = png_read_max_pixel_depth(png_ptr); + /* This value is stored in png_struct and double checked in the row read * code. */