diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index 9e134746bc..9e11981b12 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -50,8 +50,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// The huffman values public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan codeLengths, ReadOnlySpan values) { - const int Length = 257; - using (IMemoryOwner huffcode = memoryAllocator.Allocate(Length)) + // We do some bounds checks in the code here to protect against AccessViolationExceptions + const int HuffCodeLength = 257; + const int MaxSizeLength = HuffCodeLength - 1; + using (IMemoryOwner huffcode = memoryAllocator.Allocate(HuffCodeLength)) { ref short huffcodeRef = ref MemoryMarshal.GetReference(huffcode.GetSpan()); ref byte codeLengthsRef = ref MemoryMarshal.GetReference(codeLengths); @@ -63,7 +65,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (short i = 1; i < 17; i++) { byte length = Unsafe.Add(ref codeLengthsRef, i); - for (short j = 0; j < length; j++) + for (short j = 0; j < length && x < MaxSizeLength; j++) { Unsafe.Add(ref sizesRef, x++) = i; } @@ -84,7 +86,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder Unsafe.Add(ref valOffsetRef, k) = (int)(si - code); if (Unsafe.Add(ref sizesRef, si) == k) { - while (Unsafe.Add(ref sizesRef, si) == k) + while (Unsafe.Add(ref sizesRef, si) == k && si < HuffCodeLength) { Unsafe.Add(ref huffcodeRef, si++) = (short)code++; } @@ -100,19 +102,20 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Generate non-spec lookup tables to speed up decoding. const int FastBits = ScanDecoder.FastBits; - ref byte fastRef = ref this.Lookahead[0]; - Unsafe.InitBlockUnaligned(ref fastRef, 0xFF, 1 << FastBits); // Flag for non-accelerated + ref byte lookaheadRef = ref this.Lookahead[0]; + const uint MaxFastLength = 1 << FastBits; + Unsafe.InitBlockUnaligned(ref lookaheadRef, 0xFF, MaxFastLength); // Flag for non-accelerated for (int i = 0; i < si; i++) { int size = Unsafe.Add(ref sizesRef, i); if (size <= FastBits) { - int c = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size); - int m = 1 << (FastBits - size); - for (int l = 0; l < m; l++) + int huffCode = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size); + int max = 1 << (FastBits - size); + for (int left = 0; left < max; left++) { - Unsafe.Add(ref fastRef, c + l) = (byte)i; + Unsafe.Add(ref lookaheadRef, huffCode + left) = (byte)i; } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs index a2f57e485a..1c9d207cd1 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs @@ -46,9 +46,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Theory] [WithFile(TestImages.Jpeg.Issues.InvalidJpegThrowsWrongException797, PixelTypes.Rgba32)] public void LoadingImage_InvalidTagLength_ShouldThrow(TestImageProvider provider) - where TPixel : struct, IPixel - { - Assert.Throws(() => provider.GetImage()); - } + where TPixel : struct, IPixel => Assert.Throws(() => provider.GetImage()); + + [Theory] + [WithFile(TestImages.Jpeg.Issues.AccessViolationException798, PixelTypes.Rgba32)] + public void LoadingImage_BadHuffman_ShouldNotThrow(TestImageProvider provider) + where TPixel : struct, IPixel => Assert.NotNull(provider.GetImage()); } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 460aceba0a..8d8a32fba3 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -168,6 +168,7 @@ namespace SixLabors.ImageSharp.Tests public const string ExifGetString750Transform = "Jpg/issues/issue750-exif-tranform.jpg"; public const string ExifGetString750Load = "Jpg/issues/issue750-exif-load.jpg"; public const string InvalidJpegThrowsWrongException797 = "Jpg/issues/Issue797-InvalidImage.jpg"; + public const string AccessViolationException798 = "Jpg/issues/Issue798-AccessViolationException.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/Input/Jpg/issues/Issue798-AccessViolationException.jpg b/tests/Images/Input/Jpg/issues/Issue798-AccessViolationException.jpg new file mode 100644 index 0000000000..6cdbfff7c5 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue798-AccessViolationException.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:85a9eabf5b14ff974b91ba9e9b0001eda56365903d92cb4eccc52afc7f2477ab +size 381