From e884bc34903df40f270a739280ff9eae9194fc40 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Dec 2017 10:23:00 +1100 Subject: [PATCH 1/2] Fix #405 --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 9 +++++++-- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 13 +++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 6 ++++++ .../Gif/issues/issue405_badappextlength252-2.gif | 3 +++ .../Gif/issues/issue405_badappextlength252.gif | 3 +++ 5 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif create mode 100644 tests/Images/Input/Gif/issues/issue405_badappextlength252.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 80ee3d130e..de22f190a5 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -155,10 +155,15 @@ namespace SixLabors.ImageSharp.Formats.Gif this.ReadComments(); break; case GifConstants.ApplicationExtensionLabel: - this.Skip(12); // No need to read. + + // The application extension length should be 11 but we've got test images that incorrectly + // set this to 252. + int appLength = stream.ReadByte(); + this.Skip(appLength); // No need to read. break; case GifConstants.PlainTextLabel: - this.Skip(13); // Not supported by any known decoder. + int plainLength = stream.ReadByte(); + this.Skip(plainLength); // Not supported by any known decoder. break; } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index a4a27bd839..eb3e184c74 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -18,6 +18,7 @@ namespace SixLabors.ImageSharp.Tests public static readonly string[] TestFiles = { TestImages.Gif.Giphy, TestImages.Gif.Rings, TestImages.Gif.Trans }; + public static readonly string[] BadAppExtFiles = { TestImages.Gif.Issues.BadAppExtLength, TestImages.Gif.Issues.BadAppExtLength_2 }; [Theory] [WithFileCollection(nameof(TestFiles), PixelTypes)] @@ -30,6 +31,7 @@ namespace SixLabors.ImageSharp.Tests imageProvider.Utility.SaveTestOutputFile(image, "gif"); } } + [Theory] [WithFileCollection(nameof(TestFiles), PixelTypes)] public void DecodeResizeAndSave(TestImageProvider imageProvider) @@ -132,5 +134,16 @@ namespace SixLabors.ImageSharp.Tests } } } + + [Theory] + [WithFileCollection(nameof(BadAppExtFiles), PixelTypes)] + public void DecodeBadApplicationExtensionLength(TestImageProvider imageProvider) + where TPixel : struct, IPixel + { + using (Image image = imageProvider.GetImage()) + { + imageProvider.Utility.SaveTestOutputFile(image, "bmp"); + } + } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 7e98e50ade..300e169ee8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -157,6 +157,12 @@ namespace SixLabors.ImageSharp.Tests public const string Trans = "Gif/trans.gif"; public const string Kumin = "Gif/kumin.gif"; + public class Issues + { + public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; + public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; + } + public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin }; } } diff --git a/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif b/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif new file mode 100644 index 0000000000..98738c5118 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue405_badappextlength252-2.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:151d394c7c57a1696fedc32116514b667ad70c4a2c7d34db7c489c17d5755eed +size 24724 diff --git a/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif b/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif new file mode 100644 index 0000000000..6fbc3713f8 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue405_badappextlength252.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0549f4500693cdf7eac8351d8cd64d1093da9e9eaaed923e8c6f00476e350f43 +size 41721 From 124b349500e790567c1a2fe8c7329b4d06c9e95a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Dec 2017 11:01:34 +1100 Subject: [PATCH 2/2] Fix #403 --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 13 ++++++++++++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Gif/issues/issue403_baddescriptorwidth.gif | 3 +++ 4 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index de22f190a5..ae20be7d5d 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -443,7 +443,8 @@ namespace SixLabors.ImageSharp.Formats.Gif var rgba = new Rgba32(0, 0, 0, 255); - for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width; x++) + // #403 The left + width value can be larger than the image width + for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width && x < rowSpan.Length; x++) { int index = indices[i]; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index eb3e184c74..76a31c1864 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -136,7 +136,7 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [WithFileCollection(nameof(BadAppExtFiles), PixelTypes)] + [WithFileCollection(nameof(BadAppExtFiles), PixelTypes.Rgba32)] public void DecodeBadApplicationExtensionLength(TestImageProvider imageProvider) where TPixel : struct, IPixel { @@ -145,5 +145,16 @@ namespace SixLabors.ImageSharp.Tests imageProvider.Utility.SaveTestOutputFile(image, "bmp"); } } + + [Theory] + [WithFile(TestImages.Gif.Issues.BadDescriptorWidth, PixelTypes.Rgba32)] + public void DecodeBadDescriptorDimensionsLength(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + provider.Utility.SaveTestOutputFile(image, "bmp"); + } + } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 300e169ee8..365aea04ae 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -161,6 +161,7 @@ namespace SixLabors.ImageSharp.Tests { 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 static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin }; diff --git a/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif b/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif new file mode 100644 index 0000000000..0dce4b0eec --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue403_baddescriptorwidth.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c108091ffddd87178378656e37a7e975aa69be56376b9105adbbc14fe8d9a010 +size 707660