- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 888
Do not attempt to decode iDAT chunks when image is fully decoded. #2926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR prevents decoding of extra iDAT chunks once the image data is fully decoded to avoid exceptions from invalid trailing zlib data.
- Introduce a hasImageDataflag to skip subsequent chunks after decoding completes
- Update ReadScanlines,DecodePixelData, andDecodeInterlacedPixelDatato early return and set the flag
- Add a new test (Issue 2924) and reference images to validate the fix
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| tests/Images/Input/Png/issues/Issue_2924.png | Add LFS pointer for the malformed PNG used in the new test | 
| tests/Images/External/ReferenceOutput/PngDecoderTests/CanDecode_Issue2924_Rgba32_Issue_2924.png | Add the expected decoded output for Issue 2924 | 
| tests/ImageSharp.Tests/TestImages.cs | Register the new Issue2924constant | 
| tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | Add CanDecode_Issue2924test case | 
| src/ImageSharp/Formats/Png/PngDecoderCore.cs | Add hasImageDataflag and guard logic to skip extra zlib chunks | 
Comments suppressed due to low confidence (2)
src/ImageSharp/Formats/Png/PngDecoderCore.cs:137
- [nitpick] The field name 'hasImageData' is a bit vague. Consider renaming it to something like 'isImageDecoded' or 'imageDataRead' to clarify its purpose.
private bool hasImageData;
tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs:730
- [nitpick] This test only checks the Rgba32 pixel type. Consider adding additional pixel type cases (e.g., Rgb24,Argb32) or using a generic theory to ensure the fix works across all supported formats.
[WithFile(TestImages.Png.Issue2924, PixelTypes.Rgba32)]
Backport #2926 to release/2.1.x
Prerequisites
Description
Fixes #2924
With the update to the chunk identification code in v3.1.8, now detects an invalid iDAT chunk containing 6 bytes of data immediately following the correct and complete data. validation code throws an exception in cases, as this information is considered critical.
The revised behavior now ensures an early return when an issue with zLib data arises after the complete decoding of pixel data.