Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit ancillary PNG chunk size. #2681

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 18 additions & 20 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ internal sealed class PngDecoderCore : IImageDecoderInternals
/// </summary>
private readonly PngCrcChunkHandling pngCrcChunkHandling;

/// <summary>
/// The maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed.
/// </summary>
private readonly int maxUncompressedLength;

/// <summary>
/// Initializes a new instance of the <see cref="PngDecoderCore"/> class.
/// </summary>
Expand All @@ -132,6 +137,7 @@ public PngDecoderCore(PngDecoderOptions options)
this.skipMetadata = options.GeneralOptions.SkipMetadata;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.pngCrcChunkHandling = options.PngCrcChunkHandling;
this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes;
}

internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly)
Expand All @@ -143,6 +149,7 @@ internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly)
this.configuration = options.GeneralOptions.Configuration;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.pngCrcChunkHandling = options.PngCrcChunkHandling;
this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -596,23 +603,7 @@ private static void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan<byte> d
private void InitializeImage<TPixel>(ImageMetadata metadata, FrameControl frameControl, out Image<TPixel> image)
where TPixel : unmanaged, IPixel<TPixel>
{
// When ignoring data CRCs, we can't use the image constructor that leaves the buffer uncleared.
if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll)
{
image = new Image<TPixel>(
this.configuration,
this.header.Width,
this.header.Height,
metadata);
}
else
{
image = Image.CreateUninitialized<TPixel>(
this.configuration,
this.header.Width,
this.header.Height,
metadata);
}
image = new Image<TPixel>(this.configuration, this.header.Width, this.header.Height, metadata);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization was causing more trouble than it was worth. Any PNGs with invalid data yielded non-deterministic results.


PngFrameMetadata frameMetadata = image.Frames.RootFrame.Metadata.GetPngMetadata();
frameMetadata.FromChunk(in frameControl);
Expand Down Expand Up @@ -1572,7 +1563,7 @@ private void ReadColorProfileChunk(ImageMetadata metadata, ReadOnlySpan<byte> da

ReadOnlySpan<byte> compressedData = data[(zeroIndex + 2)..];

if (this.TryDecompressZlibData(compressedData, out byte[] iccpProfileBytes))
if (this.TryDecompressZlibData(compressedData, this.maxUncompressedLength, out byte[] iccpProfileBytes))
{
metadata.IccProfile = new IccProfile(iccpProfileBytes);
}
Expand All @@ -1582,9 +1573,10 @@ private void ReadColorProfileChunk(ImageMetadata metadata, ReadOnlySpan<byte> da
/// Tries to decompress zlib compressed data.
/// </summary>
/// <param name="compressedData">The compressed data.</param>
/// <param name="maxLength">The maximum uncompressed length.</param>
/// <param name="uncompressedBytesArray">The uncompressed bytes array.</param>
/// <returns>True, if de-compressing was successful.</returns>
private unsafe bool TryDecompressZlibData(ReadOnlySpan<byte> compressedData, out byte[] uncompressedBytesArray)
private unsafe bool TryDecompressZlibData(ReadOnlySpan<byte> compressedData, int maxLength, out byte[] uncompressedBytesArray)
{
fixed (byte* compressedDataBase = compressedData)
{
Expand All @@ -1604,6 +1596,12 @@ private unsafe bool TryDecompressZlibData(ReadOnlySpan<byte> compressedData, out
int bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length);
while (bytesRead != 0)
{
if (memoryStreamOutput.Length > maxLength)
{
uncompressedBytesArray = Array.Empty<byte>();
return false;
}

memoryStreamOutput.Write(destUncompressedData[..bytesRead]);
bytesRead = inflateStream.CompressedStream.Read(destUncompressedData, 0, destUncompressedData.Length);
}
Expand Down Expand Up @@ -1746,7 +1744,7 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan<byt
/// <returns>The <see cref="bool"/>.</returns>
private bool TryDecompressTextData(ReadOnlySpan<byte> compressedData, Encoding encoding, [NotNullWhen(true)] out string? value)
{
if (this.TryDecompressZlibData(compressedData, out byte[] uncompressedData))
if (this.TryDecompressZlibData(compressedData, this.maxUncompressedLength, out byte[] uncompressedData))
{
value = encoding.GetString(uncompressedData);
return true;
Expand Down
6 changes: 6 additions & 0 deletions src/ImageSharp/Formats/Png/PngDecoderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,10 @@ public sealed class PngDecoderOptions : ISpecializedDecoderOptions
/// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG.
/// </summary>
public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical;

/// <summary>
/// Gets the maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed.
/// Defaults to 8MB
/// </summary>
public int MaxUncompressedAncillaryChunkSizeBytes { get; init; } = 8 * 1024 * 1024; // 8MB
}
19 changes: 19 additions & 0 deletions tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -672,4 +672,23 @@ public void Decode_Issue2666()
string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Png.Issue2666));
using Image image = Image.Load(path);
}

[Theory]

[InlineData(TestImages.Png.Bad.BadZTXT)]
[InlineData(TestImages.Png.Bad.BadZTXT2)]
public void Decode_BadZTXT(string file)
{
string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file));
using Image image = Image.Load(path);
}

[Theory]
[InlineData(TestImages.Png.Bad.BadZTXT)]
[InlineData(TestImages.Png.Bad.BadZTXT2)]
public void Info_BadZTXT(string file)
{
string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file));
_ = Image.Identify(path);
}
}
4 changes: 3 additions & 1 deletion tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,10 @@ public static class Bad
// Invalid color type.
public const string ColorTypeOne = "Png/xc1n0g08.png";
public const string ColorTypeNine = "Png/xc9n2c08.png";

public const string FlagOfGermany0000016446 = "Png/issues/flag_of_germany-0000016446.png";

public const string BadZTXT = "Png/issues/bad-ztxt.png";
public const string BadZTXT2 = "Png/issues/bad-ztxt2.png";
}
}

Expand Down
3 changes: 3 additions & 0 deletions tests/Images/Input/Png/issues/bad-ztxt.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions tests/Images/Input/Png/issues/bad-ztxt2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.