Skip to content

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

JimBobSquarePants
Copy link
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

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.

Copy link

@Copilot Copilot AI left a 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 hasImageData flag to skip subsequent chunks after decoding completes
  • Update ReadScanlines, DecodePixelData, and DecodeInterlacedPixelData to 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 Issue2924 constant
tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs Add CanDecode_Issue2924 test case
src/ImageSharp/Formats/Png/PngDecoderCore.cs Add hasImageData flag 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)]

@JimBobSquarePants JimBobSquarePants merged commit c2a8d6f into main May 28, 2025
22 checks passed
@JimBobSquarePants JimBobSquarePants deleted the js/v4-issue-2924 branch May 28, 2025 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SixLabors.ImageSharp.ImageFormatException: Bad method for ZLIB header: cmf=3 after updating to 3.1.8
1 participant