Skip to content

Harden metadata and chunk validation - #908

Open
midoharazz9002 wants to merge 1 commit into
pnggroup:libpng18from
midoharazz9002:harden-metadata-validation
Open

Harden metadata and chunk validation#908
midoharazz9002 wants to merge 1 commit into
pnggroup:libpng18from
midoharazz9002:harden-metadata-validation

Conversation

@midoharazz9002

Copy link
Copy Markdown

Summary

This patch hardens several metadata and chunk validation paths.

Changes

  • iCCP: preserve the final zlib status from png_inflate_read() and require Z_STREAM_END when the declared profile output is exhausted.
  • tRNS: reject num_trans values outside the valid palette range before updating the PNG info state.
  • fcTL: correct the chunk length constraint from an impossible 25..26 range to 26..26.
  • eXIf: reject negative num_exif values before converting the length to png_uint_32.

Verification

  • Release build with PNG_TESTS=ON: successful
  • ctest: 37/37 passed
  • Regression tests reproduce the iCCP and tRNS issues on pristine HEAD and pass with the patch.

The changes are limited to validation/error handling and do not alter valid input handling.

@midoharazz9002

Copy link
Copy Markdown
Author

Hello is there any update.

@mohammidharaz

Copy link
Copy Markdown

Hi, just following up on PR #908.
The changes are ready for review, and the regression tests pass locally.
Thanks!

@jbowler

jbowler commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Lot's of changes so any reasonable review will take weeks. I wrote a review but github deleted it, summary:

iCCP: not reviewed
tRNS: looks wrong
fcTL: clearly right and exists in 1.6 so I fingered @ProgramMax, but never posted of course.
exIF: wrong; IRC confusing Glenn code but clearly wrong

When I say "wrong" I mean your PR is wrong, not the code.

@mohammidharaz

Copy link
Copy Markdown

Thanks for the review. I understand that fcTL is correct.

Could you please clarify what is wrong with the tRNS and eXIf changes, and what behavior you would expect instead? I want to make sure the follow-up patch matches libpng's intended API semantics.

I'll leave the iCCP change unchanged for now until it's reviewed.

@jbowler

jbowler commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

iCCP: needs a verifiable bug, not a fix.
tRNS: I did a quick analysis and github deleted it so I've leave it for someone else.
exIF: the code is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants