From ede2f2d2d1e567dea74a44a482099302af9ed14d Mon Sep 17 00:00:00 2001 From: Brian Popow <38701097+brianpopow@users.noreply.github.com> Date: Fri, 7 Jun 2024 11:58:40 +0200 Subject: [PATCH 1/2] GifDecoder: Limit lzw bits to a maximum of 12 bits (#2744) * Limit lzw bits to a maximum of 12 bits, fixes issue #2743 * Dispose currentLocalColorTable in final block * Revert "Dispose currentLocalColorTable in final block" This reverts commit 35ea961552dc9a827c6b6119d9ba26b1dd8292fc. * Don't throw; return. --------- Co-authored-by: James Jackson-South --- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 11 ++++++--- .../Formats/Gif/GifDecoderTests.cs | 23 +++++++++++-------- tests/ImageSharp.Tests/TestImages.cs | 1 + ...2012BadMinCode_Rgba32_issue2012_drona1.png | 3 +++ .../00.png | 3 +++ .../01.png | 3 +++ .../02.png | 3 +++ .../03.png | 3 +++ .../04.png | 3 +++ .../05.png | 3 +++ .../06.png | 3 +++ .../07.png | 3 +++ tests/Images/Input/Gif/issues/issue_2743.gif | 3 +++ 13 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/00.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/01.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/02.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/03.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/04.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/05.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/06.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/07.png create mode 100644 tests/Images/Input/Gif/issues/issue_2743.gif diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 4d282f26c..aecd8e5b4 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -19,6 +19,11 @@ internal sealed class LzwDecoder : IDisposable /// private const int MaxStackSize = 4096; + /// + /// The maximum bits for a lzw code. + /// + private const int MaximumLzwBits = 12; + /// /// The null code. /// @@ -73,12 +78,12 @@ internal sealed class LzwDecoder : IDisposable // It is possible to specify a larger LZW minimum code size than the palette length in bits // which may leave a gap in the codes where no colors are assigned. // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression - if (minCodeSize < 2 || clearCode > MaxStackSize) + if (minCodeSize < 2 || minCodeSize > MaximumLzwBits || clearCode > MaxStackSize) { // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided // color palette but we won't bother since the image is most likely corrupted. - GifThrowHelper.ThrowInvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); + return; } // The resulting index table length. @@ -245,7 +250,7 @@ internal sealed class LzwDecoder : IDisposable // It is possible to specify a larger LZW minimum code size than the palette length in bits // which may leave a gap in the codes where no colors are assigned. // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression - if (minCodeSize < 2 || clearCode > MaxStackSize) + if (minCodeSize < 2 || minCodeSize > MaximumLzwBits || clearCode > MaxStackSize) { // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 6399e1193..ee108b9d9 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -296,15 +296,9 @@ public class GifDecoderTests public void Issue2012BadMinCode(TestImageProvider provider) where TPixel : unmanaged, IPixel { - Exception ex = Record.Exception( - () => - { - using Image image = provider.GetImage(); - image.DebugSave(provider); - }); - - Assert.NotNull(ex); - Assert.Contains("Gif Image does not contain a valid LZW minimum code.", ex.Message); + using Image image = provider.GetImage(); + image.DebugSave(provider); + image.CompareToReferenceOutput(provider); } // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 @@ -318,4 +312,15 @@ public class GifDecoderTests image.DebugSave(provider); image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2743 + [Theory] + [WithFile(TestImages.Gif.Issues.BadMaxLzwBits, PixelTypes.Rgba32)] + public void IssueTooLargeLzwBits(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + image.DebugSaveMultiFrame(provider); + image.CompareToReferenceOutputMultiFrame(provider, ImageComparer.Exact); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index a45931e29..7aaaac6f8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -516,6 +516,7 @@ public static class TestImages public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; public const string BadDescriptorWidth = "Gif/issues/issue403_baddescriptorwidth.gif"; + public const string BadMaxLzwBits = "Gif/issues/issue_2743.gif"; public const string DeferredClearCode = "Gif/issues/bugzilla-55918.gif"; public const string Issue1505 = "Gif/issues/issue1505_argumentoutofrange.png"; public const string Issue1530 = "Gif/issues/issue1530.gif"; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png new file mode 100644 index 000000000..cdba9277b --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a3a24c066895fd3a76649da376485cbc1912d6a3ae15369575f523e66364b3b6 +size 141563 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/00.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/00.png new file mode 100644 index 000000000..923fbc122 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/00.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:800d1ec2d7c7c99d449db1f49ef202cf18214016eae65ebc4216d6f4b1f4d328 +size 537 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/01.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/01.png new file mode 100644 index 000000000..6c2134d8b --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:94dcd97831b16165f3331e429d72d7ef546e04038cab754c7918f9cf535ff30a +size 542 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/02.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/02.png new file mode 100644 index 000000000..6f50397ea --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/02.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ec1a589a8fae1b17a82b70a9583ea2ee012a476b1fa8fdba27fee2b7ce0403b2 +size 540 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/03.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/03.png new file mode 100644 index 000000000..82061ba0a --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/03.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0c8751f4fafd5c56066dbb8d64a3890fc420a3bd66881a55e309ba274b6d14e4 +size 542 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/04.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/04.png new file mode 100644 index 000000000..8902eb824 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/04.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b78516c9874cb15de4c4b98ed307e8105d962fc6bfa7aa3490b2c7e13b455a2d +size 544 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/05.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/05.png new file mode 100644 index 000000000..82061ba0a --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/05.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0c8751f4fafd5c56066dbb8d64a3890fc420a3bd66881a55e309ba274b6d14e4 +size 542 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/06.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/06.png new file mode 100644 index 000000000..6f50397ea --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/06.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ec1a589a8fae1b17a82b70a9583ea2ee012a476b1fa8fdba27fee2b7ce0403b2 +size 540 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/07.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/07.png new file mode 100644 index 000000000..75cf685e4 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueTooLargeLzwBits_Rgba32_issue_2743.gif/07.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:489642f0c81fd12e97007fe6feb11b0e93e351199a922ce038069a3782ad0722 +size 135 diff --git a/tests/Images/Input/Gif/issues/issue_2743.gif b/tests/Images/Input/Gif/issues/issue_2743.gif new file mode 100644 index 000000000..4ce61340d --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2743.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4be51cb9c258a6518d791ad2810fa0d71449805a5d5a8f95dcc7da2dc558ed73 +size 166413 From af007fa1cdb0171037e4f36ef72ca7850d610cf6 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 11 Jun 2024 14:38:30 +1000 Subject: [PATCH 2/2] Allow skipping bad metadata using identify --- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 2 +- .../Formats/Gif/GifMetadataTests.cs | 26 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index aecd8e5b4..19f646b59 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -255,7 +255,7 @@ internal sealed class LzwDecoder : IDisposable // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided // color palette but we won't bother since the image is most likely corrupted. - GifThrowHelper.ThrowInvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); + return; } int codeSize = minCodeSize + 1; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifMetadataTests.cs index fb4445cda..f9c56e05e 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifMetadataTests.cs @@ -35,7 +35,7 @@ public class GifMetadataTests RepeatCount = 1, ColorTableMode = GifColorTableMode.Global, GlobalColorTable = new[] { Color.Black, Color.White }, - Comments = new List { "Foo" } + Comments = ["Foo"] }; GifMetadata clone = (GifMetadata)meta.DeepClone(); @@ -126,7 +126,7 @@ public class GifMetadataTests public async Task Identify_VerifyRatioAsync(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit) { TestFile testFile = TestFile.Create(imagePath); - using MemoryStream stream = new(testFile.Bytes, false); + await using MemoryStream stream = new(testFile.Bytes, false); ImageInfo image = await GifDecoder.Instance.IdentifyAsync(DecoderOptions.Default, stream); ImageMetadata meta = image.Metadata; Assert.Equal(xResolution, meta.HorizontalResolution); @@ -152,7 +152,7 @@ public class GifMetadataTests public async Task Decode_VerifyRatioAsync(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit) { TestFile testFile = TestFile.Create(imagePath); - using MemoryStream stream = new(testFile.Bytes, false); + await using MemoryStream stream = new(testFile.Bytes, false); using Image image = await GifDecoder.Instance.DecodeAsync(DecoderOptions.Default, stream); ImageMetadata meta = image.Metadata; Assert.Equal(xResolution, meta.HorizontalResolution); @@ -214,4 +214,24 @@ public class GifMetadataTests Assert.Equal(frameDelay, gifFrameMetadata.FrameDelay); Assert.Equal(disposalMethod, gifFrameMetadata.DisposalMethod); } + + [Theory] + [InlineData(TestImages.Gif.Issues.BadMaxLzwBits, 8)] + [InlineData(TestImages.Gif.Issues.Issue2012BadMinCode, 1)] + public void Identify_Frames_Bad_Lzw(string imagePath, int framesCount) + { + TestFile testFile = TestFile.Create(imagePath); + using MemoryStream stream = new(testFile.Bytes, false); + + ImageInfo imageInfo = Image.Identify(stream); + + Assert.NotNull(imageInfo); + GifMetadata gifMetadata = imageInfo.Metadata.GetGifMetadata(); + Assert.NotNull(gifMetadata); + + Assert.Equal(framesCount, imageInfo.FrameMetadataCollection.Count); + GifFrameMetadata gifFrameMetadata = imageInfo.FrameMetadataCollection[imageInfo.FrameMetadataCollection.Count - 1].GetGifMetadata(); + + Assert.NotNull(gifFrameMetadata); + } }